Error:  Too many list items: Attribute geolocation_routing_policy supports 1 item maximum, but config has 2 declared

What happened?
In a Route53 Record, I was trying to set a Geo Location Policy with more than one country, but it failed with the following error:
Too many list items: Attribute geolocation_routing_policy supports 1 item maximum, but config has 2 declared. Examine values at 'Record.GeolocationRoutingPolicies'.
Expected Behavior
Pulumi should be able to create a Geo Location record with multiple countries.
Steps to reproduce
This is my sample code:
  geolocation_routing_policies=[
        aws.route53.RecordGeolocationRoutingPolicyArgs(
            country="DE",
        ),
        aws.route53.RecordGeolocationRoutingPolicyArgs(
            country="FR",
        ),
    ],

Output of pulumi about
pulumi about
CLI
Version      3.72.2
Go Version   go1.20.5
Go Compiler  gc
Plugins
NAME    VERSION
aws     5.41.0
python  unknown
Host
OS       darwin
Version  13.3.1
Arch     arm64
Additional context
The argument is called Policies but it errors out if more than 1 is specified.
Contributing
Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

Hi @mkreder - this is due to a change around 5 months ago in the upstream terraform provider:
https://github.com/hashicorp/terraform-provider-aws/commit/71649e80b60eb8aca66e81601d70891eaf41ab56
this provider changed their attribute to accept only entry in the array, which would have caused a breaking change in our provider (as you note, it's named policies which is due to it formerly being an array. Supporting their MaxItems = 1 would have renamed the attribute to policy, thus breaking existing programs). Though we allow multiple to be specified, the upstream provider catches this at validation time, and throws the error you see here.
I'm going to mark this as working as intended, though I do agree the experience can be a bit confusing.

@kpitzen is there any workaround to be able to specify more than one country? should I open an issue with terraform-provider-aws? The AWS Management console supports specifying more than one country/region in Geo location records.

I think you are right. Based on AWS Management console behavior, a separate record must be created for each region.


feat: re-enable new color filtering palette

Warning
Blocked by https://github.com/artsy/gravity/pull/15407 + a full artwork reindex, but otherwise ready for review

The type of this PR is: Feature
This PR solves FX-4001
Description
This reverts the revert in 2b25e68979007aa2698e4e43e08dae82f34e1128, in order to re-enable the updated color filtering UIs, with an updated palette of color swatches to choose from.
(Eventually we'll PR a change to drop support for the old colors).

😆 🤞🏽

Confirmed these colors are available in production now:

Gonna go ahead and merge.


redesigned Sea of Blood - suppress neutral events in deck builder

none of the neutral events should be shown, as they do not comply with agenda restrictions.

fixed.


System.NullReferenceException: Object reference not set to an instance of an object.

I get the NullReferenceException if the job is not added using .AddQuartzJob<ExecutionWorkerJobsMonitorJob>()
The error is as follows:
Job DEFAULT.ExecutionWorkerJobsMonitorJob threw an exception.
Quartz.SchedulerException: Job threw an unhandled exception.
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at Quartz.Core.JobRunShell.Run(CancellationToken cancellationToken)
   --- End of inner exception stack trace --- [See nested exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at Quartz.Core.JobRunShell.Run(CancellationToken cancellationToken)]

The AddSilkierQuartz method is called with assembly list.
The job is properly decorated with SilkierQuartzAttribute
I expect the job to be discovered and triggered every 10 seconds
I can see the job with trigger but every time they fire I get the Object reference not set to an instance of an object. error.
I am using Microsoft SQLite as store
.Net 7, with EFCore
SilkierQuartz 5.0.356
If I add .AddQuartzJob() to register the job in services collection then the error does not happen.
please find parts of the code below:
public class Startup
    {
        private readonly IConfiguration _configuration;
        private readonly IWebHostEnvironment _environment;

        public Startup(IConfiguration config, IWebHostEnvironment environment)
        {
            _configuration = config;
            _environment = environment;
        }

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddOptions();

            services.AddFeatureManagement(_configuration);
            services.AddLocalization(options =>
            {
                options.ResourcesPath = "Resources";
            });

            services.AddLazyCache();
            services.AddSerialization();
            services.AddDatabase(_configuration);
            services.AddApplicationLayer(_configuration);
            services.AddApplicationServices(_configuration);

            services
                .AddMvc()
                .AddJsonOptions(o =>
                {
                    o.JsonSerializerOptions.PropertyNamingPolicy = null;
                    o.JsonSerializerOptions.DictionaryKeyPolicy = null;
                });

            services.AddFluentValidationAutoValidation();
            services.AddValidatorsFromAssemblies(
                AssemblyHelper.GetAssemblies());

            services.AddBackgroundWorker(_configuration);
        }

        public void Configure(IApplicationBuilder app, IWebHostEnvironment env, IHostApplicationLifetime appLifetime)
        {
            app.UseExceptionHandling(env);
            app.UseHttpsRedirection();
            app.UseAuthentication();
            app.UseStaticFiles();
            app.UseRouting();
            app.UseAuthorization();
            app.UseSilkierQuartz();
            app.UseEndpoints();
        }

    }

internal static IServiceCollection AddBackgroundWorker(this IServiceCollection services, IConfiguration config)
        {
            var quartzConfiguration = config.GetSection("Quartz");

            var referencedAssemblies = AssemblyHelper
                .GetAssembliesFromExecutionFolder(
                    Assembly.GetExecutingAssembly());

            services.AddSilkierQuartz(options =>
                {

                    options.EnableEditor = false;
                    options.VirtualPathRoot = "/quartz";
                    options.UseLocalTime = true;
                    options.DefaultDateFormat = "yyyy-MM-dd";
                    options.DefaultTimeFormat = "HH:mm:ss";
                    options.CronExpressionOptions = new CronExpressionDescriptor.Options()
                    {
                        DayOfWeekStartIndexZero = false, //Quartz uses 1-7 as the range
                        Use24HourTimeFormat = true
                    };
                },
                authenticationOptions =>
                {
                    authenticationOptions.AccessRequirement = SilkierQuartzAuthenticationOptions.SimpleAccessRequirement.AllowAnonymous;
                },
                stdSchedulerFactoryOptions =>
                {
                    stdSchedulerFactoryOptions.Add(quartzConfiguration
                        .Get<Dictionary<string, string>>()
                        .ToNameValueCollection());
                },
                () => referencedAssemblies
            );

            return services;

        }

My appsettings.json part related to Quartz

"Quartz": {
    "quartz.scheduler.instanceName": "Scheduler",
    "quartz.scheduler.instanceId": "AUTO",
    "quartz.threadPool.type": "Quartz.Simpl.SimpleThreadPool, Quartz",
    "quartz.threadPool.threadCount": "10",
    "quartz.serializer.type": "json",
    "quartz.jobStore.clustered": false,
    "quartz.jobStore.useProperties": false,
    "quartz.jobStore.type": "Quartz.Impl.AdoJobStore.JobStoreTX, Quartz",
    "quartz.jobStore.driverDelegateType": "Quartz.Impl.AdoJobStore.SQLiteDelegate, Quartz",
    "quartz.jobStore.tablePrefix": "QRTZ_",
    "quartz.jobStore.dataSource": "default",
    "quartz.dataSource.default.provider": "SQLite-Microsoft",
    "quartz.dataSource.default.connectionString": "DataSource=C:\\datamigration\\db\\Datamigration.db",
    "quartz.plugin.recentHistory.storeType": "Quartz.Plugins.RecentHistory.Impl.InProcExecutionHistoryStore, Quartz.Plugins.RecentHistory",
    "quartz.plugin.recentHistory.type": "Quartz.Plugins.RecentHistory.ExecutionHistoryPlugin, Quartz.Plugins.RecentHistory"
  },

[SilkierQuartz(10, "Data migration job monitor", "Executes data migration jobs taking care of dependencies", TriggerGroup = "SilkierQuartz")]
[DisallowConcurrentExecution]
[PersistJobDataAfterExecution]
public class ExecutionWorkerJobsMonitorJob : IJob
{
    private readonly ILogger<ExecutionWorkerJobsMonitorJob> _logger;
    private readonly IDatamigrationDbContext _ctx;
    private readonly ISchedulerFactory _schedulerFactory;

    public ExecutionWorkerJobsMonitorJob(
        ISchedulerFactory schedulerFactory,
        ILogger<ExecutionWorkerJobsMonitorJob> logger,
        IDatamigrationDbContext ctx)
    {
        _schedulerFactory = schedulerFactory ?? throw new ArgumentNullException(nameof(schedulerFactory));
        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
        _ctx = ctx ?? throw new ArgumentNullException(nameof(ctx));
    }

    public async Task Execute(IJobExecutionContext context)
    {
        _logger.LogDebug("ExecutionWorkerJobsMonitorJob executed");

        ArgumentNullException.ThrowIfNull(context, nameof(context));
        var scheduler = await _schedulerFactory.GetScheduler(context.CancellationToken);
}
}

UPDATE:
It is important to mention that ExecutionWorkerJobsMonitorJob  is defined in an external assembly.
When I define a job in the same assembly as the quartz host the job runs without problems.
I was trying to do the same on example project but cannot reproduce - external assembly jobs run without problems
Need to check if db storage has anything to do about it.
UPDATE2:
I changed the example project to use persistent storage on SQLServer
I get the same System.NullReferenceException: Object reference not set to an instance of an object.
It seems the issue is related to job store with persistence
Working example: https://github.com/swidz/SilkierQuartz
Job that returns NullReferenceException is called HelloJobAutoExt in TestJobs assembly (project)

Started a discussion on Quartz.Net discussion group Help needed - System.NullReferenceException: Object reference not set to an instance of an object

For posterity,
I have found the solution. The issue was how assembly list was loaded that was passed to services.AddSilkierQuartz
To create that list, I used a helper method that internally used System.Reflection.Assembly.Load(assemblyName)
or System.Reflection.Assembly.LoadFile(assemblyPath)
but instead it should use
System.Runtime.Loader.AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName) or
System.Runtime.Loader.AssemblyLoadContext.Default.LoadFromAssemblyPath(assemblyPath).
References:
About System.Runtime.Loader.AssemblyLoadContext
Regards,
Sebastian


Double tap zoom does not work with Apple Maps on iOS.

NOTE
I have not verified whether or not double tap zoom works on iOS using Google as a provider. As far as I know it may be an underlying issue that affects both providers.
Is this a bug report?
Yes.
Have you read the Installation Instructions?
Yes.
Environment
    react: 16.6.3
    react-native: 0.57.8
    react-native-maps: 0.23.0

Target platform:
iOS, AppleMaps
Tested platforms:
Google Pixel 2, iPhone 6, iPhone 8, iPhone X.
Steps to Reproduce
(steps 3-5 are optional and are intended to illustrate the fact that this feature works on Android and not on iOS)

Clone reproducible demo to your local environment.
cd into it and install dependencies (yarn)
Add your own Google Maps API key in android/app/src/main/res/values.strings.xml.
Open the application in an Android phone or an Android emulator (I used Google Pixel 2).
Attempt to zoom using the one following method:
5.1. Double tap the screen, but do not lift your finger from the glass after the second tap.
5.2. With your finger still resting on the screen, slide up or down to respectively zoom in or out of the map.
Now open the application in an iPhone or a simulator (I tried iPhone 6 physical and iPhone 6, 8 and X simulators).
Attempt to zoom using the one finger method described in step 5.

Expected Behavior
The expected behavior would be for the iPhones to be able to zoom in or out using the one finger method. This method does work if you do it directly on Apple Maps.
It seems to work for the Android phones I have tested but not for the iPhones.
Actual Behavior
Unfortunately it does not seem like the double tap zoom method works on any iPhones.
Reproducible Demo
https://github.com/Sakarisson/react-native-maps-double-tap-zoom-repro

I can verify that it works in iOS with provider google. But it does not if you use provider apple maps. But this gesture is supported in apple maps, so I agree that I would expect this to work in RNM using apple maps.

Closing this issue because it has been inactive for a while. If you think it should still be opened let us know why. Thanks!

Indeed, the issue seems to have been fixed at some point between my report and now :)


too much rocksdb log

the LOG file of rocksdb is big ,how to set rocksdb config like info_log_level and keep_log_file_num

Thanks for reporting the issue. It has been fixed in the Master branch.


(AzureCXP) Fixes #1707

Also, standardizing punctuation inside the table.

Docs Build status updates of commit dc54210:
:clock10: Full build: the average full build time is 7 min(s) 10 sec(s), this is based on the last 314 build(s) for this repository.

Docs Build status updates of commit dc54210:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

docs-ref-conceptual/format-output-azure-cli.md
:white_check_mark:Succeeded
View (azure-cli-latest) View (azure-cli-2017-03-09-profile) View (azure-cli-2018-03-01-hybrid)

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

#sign-off


Fixes #379 #434

This PR accomplishes the following:

Adds type support for custom component builders with props for columns in the Columns type, for things like cellComponent, headerCellComponent, etc, while retaining support for StyledComponents (see ComponentBuilder)
Fixes group headers appearance in responsive view
Makes custom sortFunctions functional in both directions, and extracts reusable comparison code into the compareEntries function, fixes group sorting
Fixes sorting on the Table storyboard
Fixes responsive view
Fixes passing in of rowProps StyledRow for non-grouped Table's
Allows for styling of the ResponsiveHeaderCell, and wraps cell in CellContainer (which contains both the ResponsiveHeaderCell if in responsive mode, and the Cell).
Change type of table data to RowEntry[]. Originally was ColumnTypes, which has no overlap with expected input. More clear typing.

:tada: This PR is included in version 1.20.11 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Subsurface Scattering post process not correctly aligned with camera view

Release Type:  Current VL.Stride Version / Gamma Version is 2021.4.10
Version: VL.Stride Version <IP_ADDRESS>8
Platform(s): Windows
Describe the bug
Subsurface Scattering Material is compiling and working but the Post-Process is misaligned to the scene view.
Ive observed this in VL.Stride but it has been suggested to be a stride related issue.
To Reproduce
Steps to reproduce the behavior:
Ive tried reproducing this in the stride editor but it the scene glitches once i add the SubsurfaceScattering Blur to the GraphicsComositor in stride editor <IP_ADDRESS>8
Ive posted a .VL file to reproduce this in VL.Stride here: https://discourse.vvvv.org/t/stride-subsurface-scattering-material/20746
Sorry there is no way for me to further test this inside the Stride Editor.
Expected behavior
The post process should overlap with the scene view correctly.
Screenshots
If applicable, add screenshots to help explain your problem (use GitHub drag & drop)

Log and callstacks
Additional context

Can someone confirm if this is working using regular Stride / the Stride Editor? its still broken on the vvvv side of things and would be a nice feature..

i gave this another try, this time in the stride editor and its also broken there for me. Ive added the effect to the graphics compositor and tried almost every material setting for each slot.

Is MSAA enabled? I remember that MSAA messed up some postfx... But not sure if it is this one.


[149] - Center cells

Platform: iOS
Build ID: 38
Version:<PHONE_NUMBER>
Internal Version: 0
Short Version: 1.0.0
Branch Name: feature/149-center-cells
Install URL: https://install.appcenter.ms/orgs/Admiral-Digital-Frameworks-App/apps/AdmiralUI-Flutter-iOS-Dev/releases/38


HELM for instruction-following models

HELM was built during the era of few-shot in-context learning. The field is moving towards instruction-tuned models intended to be used in a zero-shot manner. We should update to HELM to support this.

Proposal Google doc

Done by HELM instruct, which evaluates instruction-following capability, and #2670, which adds instructions for instruction-following models on the current evaluations.


Updating css for new changes

this change also refer to the dark-light mode.

thanks for your changes.. it's perfect

close #17


htb-toolkit: init at unstable-2024-01-17

Description of changes
htb-toolkit: init at unstable-2024-01-17

Things done

Built on platform(s)

[X] x86_64-linux
[ ] aarch64-linux
[X] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[X] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[X] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[X] Tested basic functionality of all binary files (usually in ./result/bin/)
24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[X] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/prs-ready-for-review/3032/3291

Related to https://github.com/NixOS/nixpkgs/issues/81418

Result of nixpkgs-review pr 281650 run on x86_64-linux 1

1 package built:

htb-toolkit

LGTM - Builds and functions as intended on my machine.


Document how to use --filter to send specific metrics

https://github.com/Stackdriver/stackdriver-prometheus-sidecar/issues/45 implements the filtering mechanism, by passing --filter=... repeatedly we can elect to send only specific metrics to stackdriver.
This doesn't seem to be documented anywhere (except in the help text). It would be nice to have a few examples in place so that someone can easily set this up.

@fabxc you had some thoughts about documenting this (or not). Can you take a look?

@jerub do the new docs in the README resolve this issue?

Considering this fixed.


what is .net core?

I'm not a newer for devlep,but i really don't know what is .net  core?
give me a simple answer...
thanks

Closing now as this is answered.


Feature Request: Panic Alarm Mode

Hi,
I'm very thankfull for this addon and frequently use it!
One of the functions that I would see as an improvement is the panic alarm. From my Eufy Security keypad I can trigger the panic alarm by pressing the red button. Once this happens the alarm entity exposed to home assistant doesn't change.
Would there be any way to expose the panic alarm state to home assistant either trough the alarm entity or a dedicated binary sensor?
Many thanks again for the integration!

done


Created a Robolectric unit test replacement for the (failing) NearbyActivity integration test

This PR replaces the almost empty integration test for NearbyActivitywith a Robolectric based unit test.  This should speed up builds and fix broken tests on master.

Test configuration on the Robolectric tests was made a little more obvious.
The application method integrating with Dagger was brought into line with the API, facilitating the return of a mock injector in tests.
Mocks were setup to isolate NearbyActivity
Fragments in NearbyActivity were given tags, to allow for verification.

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@9a5bff4). Click here to learn what that means.
The diff coverage is 0%.

@@           Coverage Diff            @@
##             master    #990   +/-   ##
========================================
  Coverage          ?   3.77%
========================================
  Files             ?     100
  Lines             ?    5240
  Branches          ?     485
========================================
  Hits              ?     198
  Misses            ?    5028
  Partials          ?      14

Impacted Files
Coverage Δ

...n/java/fr/free/nrw/commons/CommonsApplication.java
25.21% <ø> (ø)

...ava/fr/free/nrw/commons/nearby/NearbyActivity.java
0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a5bff4...ce32505. Read the comment docs.

Awesome work @psh! This would help us write more tests. :)


Referenced Microsoft Virtual Academy videos no longer active

When I click on referenced Microsoft Virtual Academy videos above, link says videos have been retired. Please update link with current video.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 5d1a57d1-5bc2-a87f-8506-08efacb9e2a8
Version Independent ID: 0033d5ce-78cc-e023-ebea-891326860d25
Content: Learn Azure Service Fabric terminology
Content Source: articles/service-fabric/service-fabric-technical-overview.md
Service: service-fabric
GitHub Login: @rwike77
Microsoft Alias: ryanwi

@dtkuo Thanks for the feedback! I have assigned the issue to the content author to evaluate and update as appropriate.

@dtkuo  The videos, and all of the MVA website, have been retired. We're working on getting the videos hosted somewhere else. When the videos are back up we'll add the links back to the docs. Sorry for the inconvenience. #please-close


chore: release 1.2.133

:robot: I have created a release *beep* *boop*
1.2.133 (2022-07-19)
Bug Fixes

helm: skip delete if unmatched resource ownership metadata (ab416ba)

This PR was generated with Release Please. See documentation.

:robot: Release is at https://github.com/werf/werf/releases/tag/v1.2.133 :sunflower:


[SPARK-32290][SQL] SingleColumn Null Aware Anti Join Optimize

What changes were proposed in this pull request?
Normally, a Null aware anti join will be planed into BroadcastNestedLoopJoin which is very time consuming, for instance, in TPCH Query 16.
select
    p_brand,
    p_type,
    p_size,
    count(distinct ps_suppkey) as supplier_cnt
from
    partsupp,
    part
where
    p_partkey = ps_partkey
    and p_brand <> 'Brand#45'
    and p_type not like 'MEDIUM POLISHED%'
    and p_size in (49, 14, 23, 45, 19, 3, 36, 9)
    and ps_suppkey not in (
        select
            s_suppkey
        from
            supplier
        where
            s_comment like '%Customer%Complaints%'
    )
group by
    p_brand,
    p_type,
    p_size
order by
    supplier_cnt desc,
    p_brand,
    p_type,
    p_size

In above query, will planed into
LeftAnti
condition Or((ps_suppkey=s_suppkey), IsNull(ps_suppkey=s_suppkey))
Inside BroadcastNestedLoopJoinExec will perform M*N, BUT if buildSide is small enough and there is only single column in NAAJ, we can always change buildSide into a HashSet, and streamedSide just need to lookup in the HashSet, then the calculation will be optimized into M*log(N).
But this optimize is only targeting on null aware anti join with single column case, because multi-column support is much more complicated, we might be able to support multi-column in future.
After apply this patch, the TPCH Query 16 performance decrease from 41mins to 30s
Why are the changes needed?
TPCH is a common benchmark for distributed compute engine, all other 21 Query works fine on Spark, except for Query 16, apply this patch will make Spark more competitive among all these popular engine. BTW, this patch has restricted rules and only apply on NotInSubquery Single Column case, which is safe enough.
Does this PR introduce any user-facing change?
No.
How was this patch tested?

Manually run org.apache.spark.sql.SQLQueryTestSuite.
Compare performance before and after applying this patch against TPCH Query 16.

@cloud-fan
Could you please have a quick look at this issue, many thanks !!

Can one of the admins verify this patch?

Can one of the admins verify this patch?

Could you update TPCDSQueryBenchmark-results.txt, too? Can the number of q16 get better?
https://github.com/apache/spark/blob/03b5707b516187aaa8012049fce8b1cd0ac0fddd/sql/core/benchmarks/TPCDSQueryBenchmark-results.txt#L101

ok to test

Test build #125871 has started for PR 29104 at commit 042ca4a.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30479/
Test PASSed.

oh, btw, thanks for the first contribution, @leanken .

oh, btw, thanks for the first contribution, @leanken .

Will reply your comments ASAP, many thanks.

Test build #125871 has finished for PR 29104 at commit 042ca4a.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
  case class NotInSubquerySingleColumnOptimizeParams(

retest this please

Could you update TPCDSQueryBenchmark-results.txt, too? Can the number of q16 get better?
https://github.com/apache/spark/blob/03b5707b516187aaa8012049fce8b1cd0ac0fddd/sql/core/benchmarks/TPCDSQueryBenchmark-results.txt#L101

I am afraid that TPCDS sqls does not have NotInSubquery case, TPCDS sqls using Not Exists instead of Not In. What i ran before is TPCH Query 16. But i am more than happy to just write TPHC benchmark code and do benchmark after this issue closed, if needed, ^_^

Ah, I see and I missed that. You said not TPCDS but TPCH, right.

the Origin 800,000,000 * 4,898 times calculation could easily cost 50~60 mins, after apply this patch, it takes only 9s.

Test build #125884 has started for PR 29104 at commit 40d7174.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30492/
Test PASSed.

Test build #125884 has finished for PR 29104 at commit 40d7174.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/125884/
Test PASSed.

@maropu Any further comments?

But this optimize is only targeting on NotInSubquery with single column case.

One question; why did you apply this optimization only in the case? The optimization itself looks more general though.

But this optimize is only targeting on NotInSubquery with single column case.

One question; why did you apply this optimization only in the case? The optimization itself looks more general though.

you mean extend it to support multi columns?

yea, we cannnot handle the case?

-- Test cases for multi-column ``WHERE a NOT IN (SELECT c FROM r ...)'':
-- | # | does subquery include null?     | do filter columns contain null? | a = c? | b = d? | row included in result? |
-- | 1 | empty                           | *                               | *      | *      | yes                     |
-- | 2 | 1+ row has null for all columns | *                               | *      | *      | no                      |
-- | 3 | no row has null for all columns | (yes, yes)                      | *      | *      | no                      |
-- | 4 | no row has null for all columns | (no, yes)                       | yes    | *      | no                      |
-- | 5 | no row has null for all columns | (no, yes)                       | no     | *      | yes                     |
-- | 6 | no                              | (no, no)                        | yes    | yes    | no                      |
-- | 7 | no                              | (no, no)                        | _      | _      | yes
multi column Not(IsNull) is much more complicated. i am afraid that the lookup code and if-else logic will be un-readable.

let me take some time to find out common pattern among single and multi column support.

hm, it might be okay to support the limited optimization as a first step if it has a huge impact on the performance of common caes. But, I think the method names should be more general and we need to leave to TODO for future work.

For example.
-- Case 4
-- (one column null, other column matches a row in the subquery result -> row not returned)
SELECT *
FROM   m
WHERE  b = 1.0 -- Matches (null, 1.0)
AND (a, b) NOT IN (SELECT *
FROM s
WHERE c IS NOT NULL) -- Matches (0, 1.0), (2, 3.0), (4, null)
;
in this case, i can't not use InternalRow(null, 1.0) to lookup in HashedRelation. I need to exclude all null column, and try found match within the not null column, which i think HashedRelation is not a suitable structure for multi-column support. But if change into multi column and need to deal with null column, which means i can't use Hash to lookup, so it will still be a M*N, that's no gona help.

let's say in streamedSide there is a record
(null, 1, null)
and buildSide is
(1, 2, 3)
(1, 1, 3)
(null, 1, 3)
if i need to confirm a Not In, i need to extract second column values, and build HashSet; what if next time streamedSide is a
(1, null, null)
in simple words, i could not rebuild a "CUBE-LIKE" HashSet for all column combinations; or I can just rollback to just compare two records in BuildSide row by row, which is still M*N.
So I think multiple column is not suitable for these Hash Optimize because its null safe complexity

hm, it might be okay to support the limited optimization as a first step if it has a huge impact on the performance of common caes. But, I think the method (& parameter) names should be more general and we need to leave to TODO for future work.

Myself also thinks of these methodName and paramsName being too long, do you have better suggestion for me, that will be great help.

For example.
-- Case 4
-- (one column null, other column matches a row in the subquery result -> row not returned)
SELECT *
FROM m
WHERE b = 1.0 -- Matches (null, 1.0)
AND (a, b) NOT IN (SELECT *
FROM s
WHERE c IS NOT NULL) -- Matches (0, 1.0), (2, 3.0), (4, null)
;
in this case, i can't not use InternalRow(null, 1.0) to lookup in HashedRelation. I need to exclude all null column, and try found match within the not null column, which i think HashedRelation is not a suitable structure for multi-column support. But if change into multi column and need to deal with null column, which means i can't use Hash to lookup, so it will still be a M*N, that's no gona help.

ping @maropu on the multi column support conclusion.

I don't see a unit test in this PR. Can you please add one. Thanks.

@agrawaldevesh thanks for your feedback, I will first consider your suggestion about doing it in optimizer. it might take some time.

Hi. @agrawaldevesh
I am afraid that putting the optimize into BroadcastHashJoinExec is not that easy.
right now, I've got
BroadcastNestedLoopJoinExec(LeftAnti with condition Or(EqualTo(a=b), IsNull(EqualTo(a=b))))
if i want to translate into BroadcastHashJoinExec, first of all i need a join key, right?
BroadcastHashJoinExec(LeftAnti joinKey(a=b), with condition)
But the EquiJoinKeys itself already break the integrity of the origin condition Or(EqualTo(a=b), IsNull(EqualTo(a=b))
Let's see what codegenAnti is like:
s"""
         |boolean $found = false;
         |// generate join key for stream side
         |${keyEv.code}
         |// Check if the key has nulls.
         |if (!($anyNull)) {
         |  // Check if the HashedRelation exists.
         |  UnsafeRow $matched = (UnsafeRow)$relationTerm.getValue(${keyEv.value});
         |  if ($matched != null) {
         |    // Evaluate the condition.
         |    $checkCondition {
         |      $found = true;
         |    }
         |  }
         |}
         |if (!$found) {
         |  $numOutput.add(1);
         |  ${consume(ctx, input)}
         |}
       """.stripMargin

antiJoin with Key will keep streamedSideRow if streamedSide key is a null, but it's totally opposite in NotInSubquery. I can certainly do some if-else check here, but it might mess up the whole BroadcastHashJoinExec Code.
Besides the streamedSide key null difference, need to go through the entire buildSide to see if there is a null key exists, that's also kind of weird.
BroadcastHashJoinExec assume that it has join key, but if i apply my NotInSubquery check here, it would like, hey, I found two key should be joined, but wait a minute, there are a tiny corner case here, so back off.
if it's up to me to choose, i won't choose to break integrity of BroadcastHashJoinExec, i would rather count NotInSubquerySingleColumn as an runtime optimize.
So, I am polling out the relative information for you guys, seeking advice till I move forward to next step.
Choose A.
Count NotInSubquerySingleColumn as runtime optimize
Choose B.
Move code into BroadcastHashJoinExec but Codegen looks tricky.
looking for your reply, many many thanks.

Hi @leanken ... Thanks for looking into this. I agree with you that pushing null awareness into the existing BroadcastHashJoin is tricky in terms of both inferring the join keys and changes to the runtime execution logic.
One alternative (lets call it "Choose C") could be to create a new Exec Node: BroadcastNullAwareHashJoinExecNode that is implementing the null-aware logic you have currently implemented in BroadcastNestedLoopJoinExec. Such a change is not too far from the current PR: it is merely moving the logic to a new static optimizer and a new runtime exec node.
Then an optimizer rule could could rewrite the BroadcastNestedLoopJoinExecNode into BroadcastNullAwareHashJoinExecNode similar to what you are doing: ie by pattern matching on "Or(EqualTo(a=b), IsNull(EqualTo(a=b))))"

Just update BroadcastHashJoinExec code change might look like with the latest commit. Hah, but you came up with a better idea about BroadcastNullAwareHashJoinExecNode. let's follow up the rest of the work.

Test build #126010 has started for PR 29104 at commit e44c516.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30623/
Test PASSed.

BroadcastNullAwareHashJoinExecNode do i need to put Single Column keyword into the naming of the new ExecNode, because as above context mentioned, this optimize is only targeting to single column not in subquery case, specify for LeftAnti join with Or(EqualTo(a=b), IsNull(EqualTo(a=b))))
Seeking for naming advice. @agrawaldevesh

BroadcastNullAwareHashJoinExecNode do i need to put Single Column keyword into the naming of the new ExecNode, because as above context mentioned, this optimize is only targeting to single column not in subquery case, specify for LeftAnti join with Or(EqualTo(a=b), IsNull(EqualTo(a=b))))

Putting Single column in the name might make this be a mouthful and later require us to change the name again when we eventually do support more than one column :-). Instead the "check for a single column" can be a part of the new optimizer rule that will actually trigger BroadcastNullAwareHashJoinExecNode.

BroadcastNullAwareHashJoinExecNode

sure, So i will do some assertion into the new ExecNode for now, and leave TODO for future works.
@agrawaldevesh @maropu thanks, I do learn a lot from your comments.

Test build #126010 has finished for PR 29104 at commit e44c516.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126010/
Test FAILed.

Test build #126048 has started for PR 29104 at commit ccc1844.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30661/
Test PASSed.

@agrawaldevesh Ping. For updated review.

Test build #126048 has finished for PR 29104 at commit ccc1844.

This patch fails PySpark pip packaging tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class BroadcastNullAwareHashJoinExec(

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126048/
Test FAILed.

I like this idea! But I'm a bit worried about adding a new physical join node for it. I need some time to think about if we can reuse the existing broadcast hash join.

I like this idea! But I'm a bit worried about adding a new physical join node for it. I need some time to think about if we can reuse the existing broadcast hash join.

sure. Always happy to see better idea, many thanks

we can always change buildSide into a HashSet, and streamedSide just need to lookup in the HashSet, then the calculation will be optimized into M*log(N).

Taking the example you give, Anti Or((ps_suppkey=s_suppkey), IsNull(ps_suppkey=s_suppkey)), it means we need to find matches that: ps_suppkey != s_suppkey and ps_suppkey not null and s_suppkey not null. Assume s_suppkey is the build side, then we collect all the non-null values of s_suppkey and create a hash set. Then we iterate all the non-null values of ps_suppkey, and return a row if we can't find a match in the hash set.
Do I understand correctly?

null

we can always change buildSide into a HashSet, and streamedSide just need to lookup in the HashSet, then the calculation will be optimized into M*log(N).

Taking the example you give, Anti Or((ps_suppkey=s_suppkey), IsNull(ps_suppkey=s_suppkey)), it means we need to find matches that: ps_suppkey != s_suppkey and ps_suppkey not null and s_suppkey not null. Assume s_suppkey is the build side, then we collect all the non-null values of s_suppkey and create a hash set. Then we iterate all the non-null values of ps_suppkey, and return a row if we can't find a match in the hash set.
Do I understand correctly?

this is one of the two case that will return a row
ps_suppkey != s_suppkey and ps_suppkey not null and s_suppkey not null
second one is
subquery is empty, then streamedSide will unconditionally return all rows.

@cloud-fan And FYI. Inside BroadcastHashJoinExec, the HashedRelation is already filtered out with NULL value, so if we want to deal with this issue inside BHJ-Exec, might need to update from HashedRelationBroadcastMode to IdentityBroadcastMode, in this case all internalRow will be kept.
// Create a mapping of key -> rows
    var numFields = 0
    while (input.hasNext) {
      val unsafeRow = input.next().asInstanceOf[UnsafeRow]
      numFields = unsafeRow.numFields()
      val rowKey = keyGenerator(unsafeRow)
      if (!rowKey.isNullAt(0)) {
        val key = rowKey.getLong(0)
        map.append(key, unsafeRow)
      }
    }
    map.optimize()
    new LongHashedRelation(numFields, map)

 while (input.hasNext) {
      val row = input.next().asInstanceOf[UnsafeRow]
      numFields = row.numFields()
      val key = keyGenerator(row)
      if (!key.anyNull) {
        val loc = binaryMap.lookup(key.getBaseObject, key.getBaseOffset, key.getSizeInBytes)
        val success = loc.append(
          key.getBaseObject, key.getBaseOffset, key.getSizeInBytes,
          row.getBaseObject, row.getBaseOffset, row.getSizeInBytes)
        if (!success) {
          binaryMap.free()
          // scalastyle:off throwerror
          throw new SparkOutOfMemoryError("There is not enough memory to build hash map")
          // scalastyle:on throwerror
        }
      }
    }

    new UnsafeHashedRelation(key.size, numFields, binaryMap)

Test build #126064 has started for PR 29104 at commit 9e03d7e.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30677/
Test PASSed.

Test build #126064 has finished for PR 29104 at commit 9e03d7e.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class BroadcastNullAwareHashJoinExec(

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126064/
Test PASSed.

@agrawaldevesh thanks for the update. I will move on to next patch ASAP. and also deal with the prototype using BroadcastHashJoinExec.
wish you guys happy weekend!

Test build #126138 has started for PR 29104 at commit dc76141.

@agrawaldevesh updated new patch and resolved your comments.
@cloud-fan could you please have a look at the BHJ implementation and let's move on for further discussion?

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30750/
Test PASSed.

Ping for update. ^_^

Test build #126138 has finished for PR 29104 at commit dc76141.

This patch fails PySpark pip packaging tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126138/
Test FAILed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30793/
Test PASSed.

@agrawaldevesh @maropu @cloud-fan
Update digest as follow.

update HashedRelation to carry inputEmpty and anyNullKeyExists information
BroadcastHashJoinExec support both codegen and non-codegen with isNullAwareAntiJoin set.
BroadcastNullAwareHashJoinExec support both codegen and non-codegen.
remove pattern match into patterns.scala
remove NotInSubqueryHashJoinParams since HashedRelation carry inputEmpty and anyNullKeyExists, and streamedKey and streamedKeyIndex is no longer needed.
update JoinSuite with hand written left anti join case.
add spark.sql.nullAware.antiJoin.optimize.use.bhj to apply optimize using BroadcastHashJoinExec
other minor code refine.

ping for update review, many thanks.

NullAwareAntiJoinSQLQueryTestSuite passed.
org.apache.spark.sql.JoinSuite passed.
Manually Test with the following config combination with not-in-unit-tests-single-column.sql all passed.
```
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "false",
  "spark.sql.codegen.wholeStage" -> "false",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "false"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "false",
  "spark.sql.codegen.wholeStage" -> "true",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "false"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "true",
  "spark.sql.codegen.wholeStage" -> "false",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "false"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "true",
  "spark.sql.codegen.wholeStage" -> "true",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "false"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "false",
  "spark.sql.codegen.wholeStage" -> "false",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "true"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "false",
  "spark.sql.codegen.wholeStage" -> "true",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "true"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "true",
  "spark.sql.codegen.wholeStage" -> "false",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "true"
),
Map(
  "spark.sql.nullAware.antiJoin.optimize.enabled" -> "true",
  "spark.sql.adaptive.enabled" -> "true",
  "spark.sql.codegen.wholeStage" -> "true",
  "spark.sql.nullAware.antiJoin.optimize.use.bhj" -> "true"
)
```

Test build #126180 has started for PR 29104 at commit 98121d4.

Test build #126180 has finished for PR 29104 at commit 98121d4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126180/
Test FAILed.

@agrawaldevesh @clou
for BroadcasthashJoinExec, how about this
if (isNullAwareAntiJoin) {
  require(leftKeys.length == 1, "leftKeys length should be 1")
  require(rightKeys.length == 1, "rightKeys length should be 1")
  require(right.output.length == 1, "null aware anti join only supports single column.")
  require(joinType == LeftAnti, "joinType must be LeftAnti.")
  require(buildSide == BuildRight, "buildSide must be BuildRight.")
  require(SQLConf.get.nullAwareAntiJoinOptimizeEnabled,
    "nullAwareAntiJoinOptimizeEnabled must be on for null aware anti join optimize.")
  require(checkCondition == "", "null aware anti join optimize condition should be empty.")

  if (broadcastRelation.value.inputEmpty) {
    return s"""
       |// singleColumn NAAJ inputEmpty(true) accept all
       |$numOutput.add(1);
       |${consume(ctx, input)}
     """.stripMargin
  } else if (broadcastRelation.value.anyNullKeyExists) {
    return s"""
       |// singleColumn NAAJ inputEmpty(false) anyNullKeyExists(true) reject all
     """.stripMargin
  }
}

if (uniqueKeyCodePath) {
  val found = ctx.freshName("found")
  s"""
     |boolean $found = false;
     |// generate join key for stream side
     |${keyEv.code}
     |// Check if the key has nulls.
     |if (!($anyNull)) {
     |  // Check if the HashedRelation exists.
     |  UnsafeRow $matched = (UnsafeRow)$relationTerm.getValue(${keyEv.value});
     |  if ($matched != null) {
     |    // Evaluate the condition.
     |    $checkCondition {
     |      $found = true;
     |    }
     |  }
     |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""}
     |if (!$found) {
     |  $numOutput.add(1);
     |  ${consume(ctx, input)}
     |}
   """.stripMargin
} else {
  val matches = ctx.freshName("matches")
  val iteratorCls = classOf[Iterator[UnsafeRow]].getName
  val found = ctx.freshName("found")
  s"""
     |boolean $found = false;
     |// generate join key for stream side
     |${keyEv.code}
     |// Check if the key has nulls.
     |if (!($anyNull)) {
     |  // Check if the HashedRelation exists.
     |  $iteratorCls $matches = ($iteratorCls)$relationTerm.get(${keyEv.value});
     |  if ($matches != null) {
     |    // Evaluate the condition.
     |    while (!$found && $matches.hasNext()) {
     |      UnsafeRow $matched = (UnsafeRow) $matches.next();
     |      $checkCondition {
     |        $found = true;
     |      }
     |    }
     |  }
     |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""}
     |if (!$found) {
     |  $numOutput.add(1);
     |  ${consume(ctx, input)}
     |}
   """.stripMargin
}

@agrawaldevesh @cloud-fan
for BroadcasthashJoinExec, how about this
if (isNullAwareAntiJoin) {
  require(leftKeys.length == 1, "leftKeys length should be 1")
  require(rightKeys.length == 1, "rightKeys length should be 1")
  require(right.output.length == 1, "null aware anti join only supports single column.")
  require(joinType == LeftAnti, "joinType must be LeftAnti.")
  require(buildSide == BuildRight, "buildSide must be BuildRight.")
  require(SQLConf.get.nullAwareAntiJoinOptimizeEnabled,
    "nullAwareAntiJoinOptimizeEnabled must be on for null aware anti join optimize.")
  require(checkCondition == "", "null aware anti join optimize condition should be empty.")

  if (broadcastRelation.value.inputEmpty) {
    return s"""
       |// singleColumn NAAJ inputEmpty(true) accept all
       |$numOutput.add(1);
       |${consume(ctx, input)}
     """.stripMargin
  } else if (broadcastRelation.value.anyNullKeyExists) {
    return s"""
       |// singleColumn NAAJ inputEmpty(false) anyNullKeyExists(true) reject all
     """.stripMargin
  }
}

if (uniqueKeyCodePath) {
  val found = ctx.freshName("found")
  s"""
     |boolean $found = false;
     |// generate join key for stream side
     |${keyEv.code}
     |// Check if the key has nulls.
     |if (!($anyNull)) {
     |  // Check if the HashedRelation exists.
     |  UnsafeRow $matched = (UnsafeRow)$relationTerm.getValue(${keyEv.value});
     |  if ($matched != null) {
     |    // Evaluate the condition.
     |    $checkCondition {
     |      $found = true;
     |    }
     |  }
     |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""}
     |if (!$found) {
     |  $numOutput.add(1);
     |  ${consume(ctx, input)}
     |}
   """.stripMargin
} else {
  val matches = ctx.freshName("matches")
  val iteratorCls = classOf[Iterator[UnsafeRow]].getName
  val found = ctx.freshName("found")
  s"""
     |boolean $found = false;
     |// generate join key for stream side
     |${keyEv.code}
     |// Check if the key has nulls.
     |if (!($anyNull)) {
     |  // Check if the HashedRelation exists.
     |  $iteratorCls $matches = ($iteratorCls)$relationTerm.get(${keyEv.value});
     |  if ($matches != null) {
     |    // Evaluate the condition.
     |    while (!$found && $matches.hasNext()) {
     |      UnsafeRow $matched = (UnsafeRow) $matches.next();
     |      $checkCondition {
     |        $found = true;
     |      }
     |    }
     |  }
     |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""}
     |if (!$found) {
     |  $numOutput.add(1);
     |  ${consume(ctx, input)}
     |}
   """.stripMargin
}

Looks great. Can you add a comment to:      |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""} part ?
Should also lead to a shorter diff.

Test build #126214 has started for PR 29104 at commit 65c51bb.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30828/
Test PASSed.

comment

@agrawaldevesh @cloud-fan
for BroadcasthashJoinExec, how about this
if (isNullAwareAntiJoin) {
  require(leftKeys.length == 1, "leftKeys length should be 1")
  require(rightKeys.length == 1, "rightKeys length should be 1")
  require(right.output.length == 1, "null aware anti join only supports single column.")
  require(joinType == LeftAnti, "joinType must be LeftAnti.")
  require(buildSide == BuildRight, "buildSide must be BuildRight.")
  require(SQLConf.get.nullAwareAntiJoinOptimizeEnabled,
    "nullAwareAntiJoinOptimizeEnabled must be on for null aware anti join optimize.")
  require(checkCondition == "", "null aware anti join optimize condition should be empty.")

  if (broadcastRelation.value.inputEmpty) {
    return s"""
       |// singleColumn NAAJ inputEmpty(true) accept all
       |$numOutput.add(1);
       |${consume(ctx, input)}
     """.stripMargin
  } else if (broadcastRelation.value.anyNullKeyExists) {
    return s"""
       |// singleColumn NAAJ inputEmpty(false) anyNullKeyExists(true) reject all
     """.stripMargin
  }
}

if (uniqueKeyCodePath) {
  val found = ctx.freshName("found")
  s"""
     |boolean $found = false;
     |// generate join key for stream side
     |${keyEv.code}
     |// Check if the key has nulls.
     |if (!($anyNull)) {
     |  // Check if the HashedRelation exists.
     |  UnsafeRow $matched = (UnsafeRow)$relationTerm.getValue(${keyEv.value});
     |  if ($matched != null) {
     |    // Evaluate the condition.
     |    $checkCondition {
     |      $found = true;
     |    }
     |  }
     |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""}
     |if (!$found) {
     |  $numOutput.add(1);
     |  ${consume(ctx, input)}
     |}
   """.stripMargin
} else {
  val matches = ctx.freshName("matches")
  val iteratorCls = classOf[Iterator[UnsafeRow]].getName
  val found = ctx.freshName("found")
  s"""
     |boolean $found = false;
     |// generate join key for stream side
     |${keyEv.code}
     |// Check if the key has nulls.
     |if (!($anyNull)) {
     |  // Check if the HashedRelation exists.
     |  $iteratorCls $matches = ($iteratorCls)$relationTerm.get(${keyEv.value});
     |  if ($matches != null) {
     |    // Evaluate the condition.
     |    while (!$found && $matches.hasNext()) {
     |      UnsafeRow $matched = (UnsafeRow) $matches.next();
     |      $checkCondition {
     |        $found = true;
     |      }
     |    }
     |  }
     |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""}
     |if (!$found) {
     |  $numOutput.add(1);
     |  ${consume(ctx, input)}
     |}
   """.stripMargin
}

Looks great. Can you add a comment to: |} ${ if (isNullAwareAntiJoin) "else { $found = true }" else ""} part ?
Should also lead to a shorter diff.

done.

Test build #126215 has started for PR 29104 at commit 0db75b3.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30829/
Test PASSed.

Test build #126214 has finished for PR 29104 at commit 65c51bb.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

canEvaluate

join.scala has restricted rules about left anti join build side

def canBuildLeft(joinType: JoinType): Boolean = {
    joinType match {
      case _: InnerLike | RightOuter => true
      case _ => false
    }
  }

  def canBuildRight(joinType: JoinType): Boolean = {
    joinType match {
      case _: InnerLike | LeftOuter | LeftSemi | LeftAnti | _: ExistenceJoin => true
      case _ => false
    }
  }

In ExtractEquiJoinKeys (in patterns.scala) there is code like this:
        case EqualTo(l, r) if canEvaluate(l, left) && canEvaluate(r, right) => Some((l, r))
        case EqualTo(l, r) if canEvaluate(l, right) && canEvaluate(r, left) => Some((r, l))

So I am wondering if it is possible that LeftAnti join can actually have the left side as the build and the right side as streaming ? Is it possible we won't optimize that case ? Do you think it makes sense to ensure that BNLJ is not present for any not-in query ? (ie this optimization should kick in always).
I am still a bit surprised that you didn't have to modify any .sql.out files because the plan would have changed from BNLJ to BHJ.

join.scala has restricted rules about left anti join build side
it seems that only InnerLike can support both buildLeft and buildRight

def canBuildLeft(joinType: JoinType): Boolean = {
    joinType match {
      case _: InnerLike | RightOuter => true
      case _ => false
    }
  }

  def canBuildRight(joinType: JoinType): Boolean = {
    joinType match {
      case _: InnerLike | LeftOuter | LeftSemi | LeftAnti | _: ExistenceJoin => true
      case _ => false
    }
  }

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126214/
Test FAILed.

In ExtractEquiJoinKeys (in patterns.scala) there is code like this:
        case EqualTo(l, r) if canEvaluate(l, left) && canEvaluate(r, right) => Some((l, r))
        case EqualTo(l, r) if canEvaluate(l, right) && canEvaluate(r, left) => Some((r, l))

So I am wondering if it is possible that LeftAnti join can actually have the left side as the build and the right side as streaming ? Is it possible we won't optimize that case ? Do you think it makes sense to ensure that BNLJ is not present for any not-in query ? (ie this optimization should kick in always).
I am still a bit surprised that you didn't have to modify any .sql.out files because the plan would have changed from BNLJ to BHJ.

.sql.out only verified the output schema and output answer, as long as the change itself could come out with right answer, the verified will pass.

In ExtractEquiJoinKeys (in patterns.scala) there is code like this:
        case EqualTo(l, r) if canEvaluate(l, left) && canEvaluate(r, right) => Some((l, r))
        case EqualTo(l, r) if canEvaluate(l, right) && canEvaluate(r, left) => Some((r, l))

So I am wondering if it is possible that LeftAnti join can actually have the left side as the build and the right side as streaming ? Is it possible we won't optimize that case ? Do you think it makes sense to ensure that BNLJ is not present for any not-in query ? (ie this optimization should kick in always).
I am still a bit surprised that you didn't have to modify any .sql.out files because the plan would have changed from BNLJ to BHJ.

I think when both canBroadcastBySize(left) and canBroadcastBySize(right) are false, the ExtractSingleColumnNullAwareAntiJoin pattern will be misMatch,  then it will still fallback to the origin BNLJ, FYI.

Test build #126215 has finished for PR 29104 at commit 0db75b3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126215/
Test PASSed.

@agrawaldevesh code updated.

Test build #126237 has started for PR 29104 at commit 20805a0.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30852/
Test PASSed.

Test build #126237 has finished for PR 29104 at commit 20805a0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126237/
Test FAILed.

Test build #126260 has started for PR 29104 at commit bd93218.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30876/
Test PASSed.

@cloud-fan resolved your comments, ping for update.

@cloud-fan @agrawaldevesh @maropu do we have conclusion for whether put NAAJ logical code in existing BroadcastHashJoinExec or in a new BroadcastNullAwareHashJoinExec ?

I'm on a fence now. We do reuse some codegen stuff in BHJ, but it also makes BHJ a bit hard to read since we put if (isNullAwareAntiJoin) in several places. On the other hand, a new node needs to duplicate some code, but seems OK as the duplication is small.
cc @maryannxue @viirya @maropu

I'm on a fence now. We do reuse some codegen stuff in BHJ, but it also makes BHJ a bit hard to read since we put if (isNullAwareAntiJoin) in several places. On the other hand, a new node needs to duplicate some code, but seems OK as the duplication is small.
cc @maryannxue @viirya @maropu

we could take multi-column support as another factor.

"no row has null for all columns" this will it even harder to use hashedRelation. because hashedRelation currently will filter rows which has any column which is null, but  we need these records.
Broadcast[Array[InternalRow]] is more suitable for multi column support, for long-term consideration, BHJ might be messed up with multi column support because it's much more complicated, if putting code into new ExecNode, it will be less Historical burden.

I'm fine with having a new class. The downside is that we need to make sure everywhere we match BHJ, we match this new class too if needed. And this might be a future maintenance concern as well.

@leanken ... How important is the isUniqueKeyCodePath in the BHJ codegen when NAAJ is present ? That is the key optimization in BHJ and I am wondering if it is even applicable to NAAJ ? The reason I ask is:

If that optimization is relevant for NAAJ, then you would have to duplicate it into the new broadcast null aware anti join node.
If it is not relevant, then you can omit it in the new node.

Basically, I think if we end up having to duplicate a lot of logic into the new node, then the new node becomes less appealing.
I can see why having a new node is a good way to future proof, but I genuinely think that we may never need to actually embellish not-in query for multiple-keys and/or the distributed case given how complex it is. Or atleast it would be great to see some real data that suggests that multiple keys and distributed build side is common enough to warrant the complexity.
So my vote would be: If the new node ends up copying a lot of code (like the isUniqueKey optimization) then we should just consider modifying the BHJ as is. That is, at this point I am leaning towards just modifying the BHJ node with a flag.
We can always refactor and move this into a new node when we actually get around to implementing more complex not-in cases.

Test build #126260 has finished for PR 29104 at commit bd93218.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126260/
Test PASSed.

@leanken ... IMHO, this is getting close and really you have done an amazing job not just trying out all the different ideas and approaches but also paying attention to testing etc.
As for my vote on the question of different node vs hacking at the BHJ: I think I am finally convinced that having a separate node is the way to go here since there is just too much difference wrt to BHJ. Your answer on isUniqueKeyCodePath was the clincher for me. It's indeed better to have a different node than making another core node harder to follow. It would also restrict the performance impact of this PR to avoid making any inadvertent regressions into core BHJ.
My last remaining request is wrt to improving the unit testing coverage. I was wondering if we can copy paste some of the not-in query tests from SubQuerySuite and in addition to testing the results, also assert that the optimization kicks in or not. Ideally it would be great to reuse those tests without duplicating them, but its better to duplicate than not test them with this special optimization !.
Thanks.

@leanken ... IMHO, this is getting close and really you have done an amazing job not just trying out all the different ideas and approaches but also paying attention to testing etc.
As for my vote on the question of different node vs hacking at the BHJ: I think I am finally convinced that having a separate node is the way to go here since there is just too much difference wrt to BHJ. Your answer on isUniqueKeyCodePath was the clincher for me. It's indeed better to have a different node than making another core node harder to follow. It would also restrict the performance impact of this PR to avoid making any inadvertent regressions into core BHJ.
My last remaining request is wrt to improving the unit testing coverage. I was wondering if we can copy paste some of the not-in query tests from SubQuerySuite and in addition to testing the results, also assert that the optimization kicks in or not. Ideally it would be great to reuse those tests without duplicating them, but its better to duplicate than not test them with this special optimization !.
Thanks.

Ok. i will do the suggestion about SubQuerySuite. and another issue need to confirm with you.
as @viirya said, how about change BroadcastNullAwareHashJoinExec into BroadcastNullAwareLeftAntiHashJoinExec since there is no other NullAwareXXXXJoin, I google a lot, it seems that NAAJ is the only null aware join.

let me collect the vote information for BHJ VS. new ExecNode.
vote for new ExecNode
@agrawaldevesh @maryannxue @viirya @leanken
vote for BHJ
None for now.
"on a fence"
@cloud-fan @maropu
it looks like "New ExecNode" is winning a little bit. ^_^

Ok. i will do the suggestion about SubQuerySuite. and another issue need to confirm with you.
as @viirya said, how about change BroadcastNullAwareHashJoinExec into BroadcastNullAwareLeftAntiHashJoinExec since there is no other NullAwareXXXXJoin, I google a lot, it seems that NAAJ is the only null aware join.

Go for it. Makes sense !.

Test build #126312 has started for PR 29104 at commit e090286.

@leanken ... IMHO, this is getting close and really you have done an amazing job not just trying out all the different ideas and approaches but also paying attention to testing etc.
As for my vote on the question of different node vs hacking at the BHJ: I think I am finally convinced that having a separate node is the way to go here since there is just too much difference wrt to BHJ. Your answer on isUniqueKeyCodePath was the clincher for me. It's indeed better to have a different node than making another core node harder to follow. It would also restrict the performance impact of this PR to avoid making any inadvertent regressions into core BHJ.
My last remaining request is wrt to improving the unit testing coverage. I was wondering if we can copy paste some of the not-in query tests from SubQuerySuite and in addition to testing the results, also assert that the optimization kicks in or not. Ideally it would be great to reuse those tests without duplicating them, but its better to duplicate than not test them with this special optimization !.
Thanks.

add case in SubquerySuite. check on both result and whether the newExecNode hit.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/30927/
Test PASSed.

Test build #126312 has finished for PR 29104 at commit e090286.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126312/
Test PASSed.

@cloud-fan @maropu need update from you so as to seal this PR. ^_^

@cloud-fan @maropu need update from you so as to seal this PR. ^_^

Specifically, @leanken wants to get your vote on whether we should have a separate execution node doing the anti join or fold that into BHJ ? Once we have clarity on that, the PR can be pruned up and it will become easier to do a final review.
(Just putting words in @leanken's mouth !)

@cloud-fan @maropu need update from you so as to seal this PR. ^_^

Specifically, @leanken wants to get your vote on whether we should have a separate execution node doing the anti join or fold that into BHJ ? Once we have clarity on that, the PR can be pruned up and it will become easier to do a final review.
(Just putting words in @leanken's mouth !)

you got me ! ^_^

Test build #126405 has started for PR 29104 at commit c8985a4.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31019/
Test PASSed.

update notes @cloud-fan @agrawaldevesh @maropu

isInputEmpty rename to isOriginInputEmpty
anyNullKeyExists rename to allNullColumnKeyExistsInOriginInput
And these two attribute to be placed inside LongHashedRelation and UnsafeHashedRelation, putting them in ByteToByteMap and LongToUnsafeRowMap is making less sense
add isNullAware mode in HashedRelationBroadcastMode, with isNullAware = true, during building relation from Iterator[InternalRow], it will not skip any null key rows, this was a prepare step for supporting multi-column
USE BHJ as only implementation and delete BroadcastNullAwareLeftAntiHashJoinExec
add allNull method inside UnsafeRow
extract common pattern from single and multi not in as follow:

isOriginInputEmpty => return all rows
allNullColumnKeyExistsInOriginInput => reject all rows
if streamedSideRow.allNull is true => drop the row
if streamedSideRow.allNull is false & findMatch in NullAwareHashedRelation => drop the row
if streamedSideRow.allNull is false & notFindMatch in NullAwareHashedRelation => return the row

Test build #126405 has finished for PR 29104 at commit c8985a4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126405/
Test FAILed.

Test build #126405 has finished for PR 29104 at commit c8985a4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

test case failed because isNullAware field is added in HashedRelationBroadcastMode. will update sql.out

Test build #126409 has started for PR 29104 at commit 96a7a3e.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31023/
Test PASSed.

Test build #126409 has finished for PR 29104 at commit 96a7a3e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126409/
Test PASSed.

Test build #126444 has started for PR 29104 at commit 187f7ce.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31058/
Test PASSed.

@agrawaldevesh updated. ^_^

Test build #126458 has started for PR 29104 at commit 051c6b7.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31072/
Test PASSed.

Test build #126458 has finished for PR 29104 at commit 051c6b7.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Test build #126444 has finished for PR 29104 at commit 187f7ce.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126458/
Test FAILed.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126444/
Test FAILed.

retest this please

@agrawaldevesh Any further comments?

No :) I am trying to figure out how to get this breaking thing. I have also
asked a bunch of other folks who know codegen more to opine here.
On Fri, Jul 24, 2020 at 12:14 AM leanken<EMAIL_ADDRESS>wrote:

@agrawaldevesh https://github.com/agrawaldevesh Any further comments?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/apache/spark/pull/29104#issuecomment-663380985, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AE44R7Y5RSRQLKULJ5TIF4LR5EYGJANCNFSM4OZUR3ZA
.

could you help trigger a retest for this patch? many thanks

retest this please

Test build #126489 has started for PR 29104 at commit 213a8cf.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31106/
Test PASSed.

Test build #126489 has finished for PR 29104 at commit 213a8cf.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126489/
Test PASSed.

Test build #126513 has started for PR 29104 at commit 0b2a565.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31132/
Test PASSed.

ensure

Thanks for your great help during the review. And i sure will make sure that the BHJ is not regressed by this change.

Test build #126513 has finished for PR 29104 at commit 0b2a565.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126513/
Test PASSed.

Merged build finished. Test PASSed.

Test build #126614 has started for PR 29104 at commit 1d223d6.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31236/
Test PASSed.

rebased community master branch, resolved your comments and change spark.sql.nullAwareAntiJoin.optimize.enabled to default true. I will ping you after all test passed.
thanks @cloud-fan

Test build #126615 has started for PR 29104 at commit a269bba.

Test build #126614 has finished for PR 29104 at commit 1d223d6.

This patch fails due to an unknown error code, -9.
This patch does not merge cleanly.
This patch adds no public classes.

Test build #126615 has finished for PR 29104 at commit a269bba.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126615/
Test FAILed.

Build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126614/
Test FAILed.

Test build #126636 has started for PR 29104 at commit 982ccbc.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31257/
Test PASSed.

@cloud-fan updated. with your suggestion, hashedRelation code diff is smaller and making more sense.

Test build #126644 has started for PR 29104 at commit 891514a.

@cloud-fan updated.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31265/
Test PASSed.

Test build #126636 has finished for PR 29104 at commit 982ccbc.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class EmptyHashedRelation extends HashedRelation with Externalizable
class EmptyHashedRelationWithAllNullKeys extends HashedRelation with Externalizable

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126636/
Test PASSed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31272/
Test PASSed.

Test build #126651 has started for PR 29104 at commit 6a4c9d9.

Test build #126652 has started for PR 29104 at commit f6f7102.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31273/
Test PASSed.

Test build #126655 has started for PR 29104 at commit 395823d.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31276/
Test PASSed.

Test build #126644 has finished for PR 29104 at commit 891514a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait NullAwareHashedRelation extends HashedRelation with Externalizable
class EmptyHashedRelation extends NullAwareHashedRelation
class EmptyHashedRelationWithAllNullKeys extends NullAwareHashedRelation

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126644/
Test PASSed.

Test build #126651 has finished for PR 29104 at commit 6a4c9d9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126651/
Test PASSed.

Test build #126652 has finished for PR 29104 at commit f6f7102.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126652/
Test PASSed.

Test build #126655 has finished for PR 29104 at commit 395823d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126655/
Test PASSed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/31293/
Test PASSed.

Test build #126672 has started for PR 29104 at commit 233eff6.

Test build #126672 has finished for PR 29104 at commit 233eff6.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126672/
Test FAILed.

github action passes, I'm merging it to master, thanks for your great work!


Prototype running kpng-windows in Containers

Currently the way we test is in process
https://jayunit100.blogspot.com/2022/03/my-dev-workflow-for-kpng-windows.html?q=windows
However, @marosset has volunteered to get kpng windows backend working in a container (host process) which would be great !
To do this we'd need to

build the container
containerize the api=... to-winkernel

https://github.com/kubernetes-sigs/kpng/pull/337

Still doing ths

this is done mark got it working.

We will pick this work up in k-sigs/windows-service-proxy


use local time instead of utc for atScheduling

I got GMT+3 time zone, so when i write
gron.Every(1 * xtime.Day).At("19:32")
i expect function will be triggered at 19:32 local time, not UTC time (which is 3 hours difference in my case).
Tests are green, looks like no side effects.

I'm considering a parameter for location, what do you think of that? :)


Insert mode put

In insert mode in VIM, you can press ctrl-r followed by a register name to paste the contents of the register.

Very cool.
I agree, the ctrl-e conflict is worrisome.  I really like that normal mode feels like Vim and insert mode feels like Atom.
Would it make sense to give vim-mode a configuration option: "Vim-Flavored Insert Mode" and "Atom-Flavored Insert Mode"?  When there's a conflict, this setting would tell whose keybindings to prefer.
I'd set mine to Atom-flavored.

This is really well done. My apologies for not seeing this sooner.

Thanks, and no worries about the timing, I understand that getting Atom 1.0 out the door made everyone quite busy. 8-)


How to solve the problem of insufficient interrupt resource conflicts (IDFGH-14267)

Answers checklist.

[X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
[X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
[X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.
v3.2-dev-306-gbeb3611ca
Espressif SoC revision.
ESP32S3
Operating System used.
Windows
How did you build your project?
Eclipse IDE
If you are using Windows, please specify command line type.
CMD
Development Kit.
ESP IDF
Power Supply used.
USB
What is the expected behavior?
The program prompts that interrupt resources are running low and need to be freed up.
But from the interrupt resources of printing, it seems that all interrupts that have already been allocated must be allocated interrupts, otherwise the system may not run. Please provide a solution
What is the actual behavior?
prompt the following info
No free interrupt inputs for SPI3 interrupt
Steps to reproduce.
I have printed out the resources allocated for interrupts, and the allocation is as follows. I still need 2 interrupt resources for other functions. How can I adjust the following resources to free up 2 interrupt resources
    intr_alloc: Connected src 39 to int 2 (cpu 0)		ETS_RTC_CORE_INTR_SOURCE
intr_alloc: Connected src 59 to int 3 (cpu 0)		ETS_SYSTIMER_TARGET2_INTR_SOURCE
intr_alloc: Connected src 79 to int 9 (cpu 0)		ETS_FROM_CPU_INTR0_SOURCE
intr_alloc: Connected src 57 to int 12 (cpu 0)		ETS_SYSTIMER_TARGET0_INTR_SOURCE
intr_alloc: Connected src 80 to int 2 (cpu 1)		ETS_FROM_CPU_INTR1_SOURCE
intr_alloc: Connected src 58 to int 3 (cpu 1)		ETS_SYSTIMER_TARGET1_INTR_SOURCE
intr_alloc: Connected src 52 to int 13 (cpu 0)		ETS_TG0_WDT_LEVEL_INTR_SOURCE
intr_alloc: Connected src 27 to int 17 (cpu 0)		ETS_UART0_INTR_SOURCE
intr_alloc: Connected src 29 to int 18 (cpu 0)		ETS_UART2_INTR_SOURCE
intr_alloc: Connected src 28 to int 19 (cpu 0)		ETS_UART1_INTR_SOURCE
intr_alloc: Connected src 16 to int 20 (cpu 0)		ETS_GPIO_INTR_SOURCE
intr_alloc: Connected src 21 to int 21 (cpu 0)		ETS_SPI2_INTR_SOURCE
intr_alloc: Connected src 38 to int 23 (cpu 0)		ETS_USB_INTR_SOURCE

Debug Logs.
19: I (1236) intr_alloc: Connected src 39 to int 2 (cpu 0)
22: I (1253) intr_alloc: Connected src 59 to int 3 (cpu 0)
27: I (1284) intr_alloc: Connected src 79 to int 9 (cpu 0)
28: I (1290) intr_alloc: Connected src 57 to int 12 (cpu 0)
29: I (1290) intr_alloc: Connected src 80 to int 2 (cpu 1)
30: I (1300) intr_alloc: Connected src 58 to int 3 (cpu 1)
33: I (1310) intr_alloc: Connected src 52 to int 13 (cpu 0)
37: I (1350) intr_alloc: Connected src 27 to int 17 (cpu 0)
41: I (1440) intr_alloc: Connected src 29 to int 18 (cpu 0)
42: I (1440) intr_alloc: Connected src 28 to int 19 (cpu 0)
59: I (1530) intr_alloc: Connected src 16 to int 20 (cpu 0)
60: I (1530) intr_alloc: Connected src 21 to int 21 (cpu 0)
81: I (2920) intr_alloc: Connected src 38 to int 23 (cpu 0)

I (9146) MQTT: rcv get_version cmd
I (9146) OTDR_IAP: OTDR version is: 0260
I (10136) example: Install panel IO
**E (10136) intr_alloc: No free interrupt inputs for SPI3 interrupt (flags 0x80E)**
E (10136) lcd_panel.io.spi: esp_lcd_new_panel_io_spi(87): adding spi device to bus failed
I (10146) gpio: GPIO[36]| InputEn: 0| OutputEn: 0| OpenDrain: 0| Pullup: 1| Pulldown: 0| Intr:0
ESP_ERROR_CHECK failed: esp_err_t 0x105 (ESP_ERR_NOT_FOUND) at 0x4200e507

More Information.
No response

The following interrupts are all connected to hardware devices and are necessary.
ETS_UART0_INTR_SOURCE
ETS_UART2_INTR_SOURCE
ETS_UART1_INTR_SOURCE
ETS_GPIO_INTR_SOURCE
ETS_SPI2_INTR_SOURCE
ETS_USB_INTR_SOURCE

Please take a look at the second part of this documentation section, suggestions 2 and 3 seem relevant to your case.

Multiple CPUs make significant changes to my solution, which is not very practical. Can the shared solution be applied to serial ports
Additionally, there are many interruptions that are reserved here. Why are they reserved and how can they be enabled
CPU 0 interrupt status:
Int  Level  Type   Status
0     1    Level  Reserved
1     1    Level  Reserved
2     1    Level  Used: RTC_CORE
3     1    Level  Used: SYSTIMER_TARGET2
4     1    Level  Reserved
5     1    Level  Reserved
6     1    Level  CPU-internal
7     1    Level  CPU-internal
8     1    Level  Reserved
9     1    Level  Used: FROM_CPU_INTR0
10     1    Edge   Free (not general-use)
11     3    Level  CPU-internal
12     1    Level  Used: SYSTIMER_TARGET0
13     1    Level  Used: TG0_WDT_LEVEL
14     7    Level  Reserved
15     3    Level  CPU-internal
16     5    Level  CPU-internal
17     1    Level  Used: UART0
18     1    Level  Used: UART2
19     2    Level  Used: UART1
20     2    Level  Used: GPIO
21     2    Level  Used: SPI2
22     3    Edge   Reserved
23     3    Level  Used: USB
24     4    Level  Reserved
25     4    Level  Reserved
26     5    Level  Free (not general-use)
27     3    Level  Reserved
28     4    Edge   Free (not general-use)
29     3    Level  CPU-internal
30     4    Edge   Reserved
31     5    Level  Reserved

Without allocating interrupts to both CPUs you will face difficulty fully utilizing everything as S3 has a lot of interrupt sources.
Most of the peripheral drivers should support allocating shared interrupts, I suggest you give that approach a try if using both CPUs is not possible.

Why are they reserved and how can they be enabled

They are reserved for misc system stuff or e.g. wifi, so enabling them wont necessary be possible. The last year we merged a79c6f7f67741fcba4ac0bc2d98749062b90c837 to give greater flexibility and free up some interrupts, so you could try upgrading to a version that includes this one and see if it helps you.

ok,thanks,   I'll upgrade to the new version and check it.


Improvement in the feedback form

Is your feature request related to a problem? Please describe.

ui of the feedback form is not upto the mark
Describe the solution you'd like

improvement in the ui of the feedback form
Describe alternatives you've considered

Demo Video/Screenshot (Mandatory )
Are you contributing under any open-source program ?

@apu52  Please assign me this issue under GSSOC'24.

Please assign this task to me under GssoC'24

Hi ,
This issue has been already resolved.
Regards
Project Mentor (GSSoC' 24)


Should the link to 'Windows Defender ATP' related in paragraph Enhanced Security?

The last control element of Enhanced Security profile from the figure is Windows Defender ATP, but the link in the detail explanation points to O365 ATP, suggest change the link to Windows Defender ATP page.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: cd1e72d8-1b89-f29c-bb0b-08593d4810a2
Version Independent ID: d62c3d7c-eca8-02ee-7615-2e0174588799
Content: Understand secure, Azure-managed workstations - Azure Active Directory
Content Source: articles/active-directory/devices/concept-azure-managed-workstation.md
Service: active-directory
Sub-service: devices
GitHub Login: @MicrosoftGuyJFlo
Microsoft Alias: joflore

@safeng510 Thanks for the comment.We are actively looking into it and will update you soon.

@safeng510 The document has been updated accordingly. Thanks for reporting this.


Added shaders on GLSL 1.2

Used glium::program to create shaders
Moved shaders code to its own sources and include it as str
Added shaders implementation on GLSL which allows run egui on old hardware
(Raspberry Pi 1/zero in game again)

It should solve #79 #80.
Another shader versions (1.1, 1.3, 1.0 ES and etc.) still missing and can be added later.

I'm not an expert in GLSL dialects but seems the shader's code from webgl1 and webgl2 can be reused for GLSLES 1.0 and 3.0 respectively.

@katyo did you test both fragment_300es.glsl and fragment_100es.glsl ?
I ask because they seem to be carbon-copied of the WebGL shaders, including statements like "WebGL doesn't come with sRGBA textures" (but does GLES?) and "WebGL doesn't support linear blending in the framebuffer" (but does GLES?).
This is particularly relevant since I recently improved the WebGL shaders: https://github.com/emilk/egui/commit/effd3c7440d9f70c0eff677871a60be539cf1113

@emilk Seems the version string is wrong for GLSL ES 1.00 shaders. According to specs it should be #version 100 without es at end. I forget to fix this.
I cannot test GLSL ES 3.0 version just now, but 1.00 works fine (with version fix).


[WIP] Support fedora and rhel7 based containers in osbs-box.py

This adds two new params for osbs-box.py:

--distro - it determines which Dockerfile to use.

By default rhel7 and fedora (latest stable, F26) are provided
More specific dockerfiles - e.g. fedora26-py3 - are possible.

--repo-url - a URL to the repo file with repos for RHEL images.

RHEL7 images fail to start due to certificate error when connecting to koji-hub

What if we use a common parent image for both fedora and rhel versions?
For instance, if fedora is chosen, part of bringing up osbs-box, a osbs-box:latest image is built locally from, say, fedora:26. If rhel is chosen, it also builds osbs-box: latest, but in this case it uses a different FROM instruction. These Dockerfiles would be minimal, perhaps even inline.
This would make the docker compose part simpler, as only a single docker-compose.yaml is needed. Just need to be careful when installing packages due to name differences, but this can be easily solved.
What do you think?

For instance, if fedora is chosen, part of bringing up osbs-box, a osbs-box:latest image is built locally from, say, fedora:26. If rhel is chosen, it also builds osbs-box: latest, but in this case it uses a different FROM instruction

I'm not sure if that would be easy and extensible. For instance, osbs-box would be useful to test out py3-only version using fedora - that would require an entirely different package set. Same for imagebuilder/buildah plugins etc.


Add issue templates

To this repo & template. I have some here https://github.com/paddyroddy/python-template/tree/main/.github/ISSUE_TEMPLATE

Fixed in #262

🤔 I'm not seeing any template when I try and open a new issue

🤔 I'm not seeing any template when I try and open a new issue, just the usual empty title and text boxes

Hmm... me neither


Seattle WA Rape & homicide records

Someone has requested help on a data project:
Submission notes
What kinds of records do we need to document clearance rates for rape and homicide?
Location
Seattle, Washington
Requirements
Data selection
How to help
Leave a comment if you'd like to help. Do you have ideas, questions, or need any more information to get started?

Our response:
"It depends how you define “clearance”—and your ability to follow a case through to prosecution. Some agencies may define their clearance rate as as arrest, but not update their numbers if the charge is dropped.
You could request a document about “clearance rates” from the police, but they may not have such a document lying around. You would also need to know how it was generated, and ideally see the underlying data.
In general it’s crime reports → arrests/charges → court cases/prosecutions.
FBI's NIBRS database may be a good starting point. It's missing a lot of data, and it's normalized so you're missing some context, but you can compare crime to arrests in the same department across specified time periods."


fix(sqli): additional regular expressions for SQL auth bypass

This PR contains new regular expressions for different kinds of auth bypasses. In an ideal world, these would be inside the operator dataset, but unfortunately, they look too much like normal sentences to accurately block.
Edit:

Contains fixes to bypasses found by me (DV2UKLP4, 2AUWUOVF, RICGZH4Q)
Contains fixes to bypasses found by @wfinn (YPWZU6PD and Q6FBUFOD)

Hi @karelorigin , thanks for sending this!

Hi!
This PR should also be able to catch a NoSQL bypass reported by @hussein98d in RQ1R4RCP, except that it's not evaluated on the URL path. Is this something I could add?

This also covers DV2UKLP4

Hi @dune73, I've updated my original comment to better outline the reports this PR applies to. That should be the complete list of reports relevant to this PR.

Thank you. We'll get to the question of RQ1R4RCP during the review.

CRS Bug Bounty PR assessment

Rules affected (list rules): FIXME
Paranoia Level addressed (1, 2, 3, 4, full or explain): FIXME
FTW passes (yes or no) : FIXME
Rule(s) picked for solution (correct or not-correct or explain) : FIXME
Risk for false positives (irrelevant, adequate, substantial or explain) : FIXME
Regular expression quality (inspirational, decent base, needs work, adequate or explain) : FIXME
Documentation (needs work, adequate or explain) : FIXME
Tests (none or some or adequate) : FIXME

Verdict (Unusable, inspirational, usable, almost perfect or perfect) : FIXME

This is not meant to be final. As a CRS dev, feel free to comment below and edit this form directly. As committer or observer, feel free to comment below with feedback and we will think about updating the assessment accordingly.

CRS TODO: add a comment on file util/regexp-assemble/data/942340.data line 43-47 on why are those there, so documentation is in one file (we can't ask this in this PR, wasn't agreed in the first place).

Hi @karelorigin !
Inevitably, we run onto conflicts after we started merging. Can you help us resolve this one?

Hey @fzipi, yeah for sure! I could rebase but that rewrites git history and will probably create a mess, so I'll try to do it via the GitHub interface. Will work on it as soon as I get the chance (might take a couple of days)

Hi @karelorigin, there seem to be still some odd conflicts, could you solve them or do you need help?

@lifeforms, I've been super busy but I'll get on this ASAP. I should be able to resolve them just fine but I'll definitely ask for help if I need it. Sorry about that!

Resolved all conflicts, should be fine merging this now :)

Looks awesome! Thanks @karelorigin for this PR! 🎉


Adopt Isolated Cluster creation to the new possibility of machine / firewall creation with DNS and NTP Servers

Since https://github.com/metal-stack/metal-api/pull/571 it is possible to specify DNS and NTP Servers during machine/firewall creation. This enables us to simplify the logic to modify the DNS and NTP Server configuration with the help of the os-metal-extension.
Instead we should adopt the machine/firewall creation process to put the DNS and NTP Servers given in the cloud profile for isolated clusters to the allocation.
This needs to be adopted in:

[ ] this repo
[ ] the os-metal-extension
[ ] machine-controller-manager-metal

If in a specific partition these parameters are defined, the parameters from the cloud profile must be of higher precedence.
@Gerrit91 @mwennrich @simcod @vknabel @robertvolkmann Please add comments if required.

Please mind the documentation: https://github.com/metal-stack/docs/blob/master/docs/src/overview/isolated-kubernetes.md


Boards: nucleo_stm32g474re does not build

Describe the bug
The devicetree was switched to use use pinctrl, but the pinctrl dtsi file is not in the tree
Please add the stm32g474r(b-c-e)tx-pinxtrl.dtsi file to rhe tree

The file exists: https://github.com/zephyrproject-rtos/hal_stm32/blob/master/dts/st/g4/stm32g474r(b-c-e)tx-pinctrl.dtsi. Can you check if modules are updated?

Oh. That is in the HAL. Is there a reason for that?