[BugFix] Fix bin/chart NPE with null values (#5174)

[qianheng-aws]

Description

Fix NullPointerException when using bin then chart with null values in the binned field.

Root cause: The bin bucket functions (SpanBucketFunction, MinspanBucketFunction, RangeBucketFunction) declared a non-nullable VARCHAR(2000) return type via ReturnTypes.VARCHAR_2000, even though they return null for null inputs. This caused Calcite's optimizer to remove the IS NOT NULL filters (added by visitAggregation's nonNullGroupMask logic) as trivially true, allowing null group keys to reach the Enumerable aggregation's TreeMap, which crashed with NPE in compareTo.

Fix:

1. Declare the return type as nullable using ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE) so that the null-filtering logic in visitAggregation works correctly
2. Add nullsLast to chart command sort operations as a defensive measure

Related Issues

Resolves #5174

Check List

• New functionality includes testing
• Commits signed per DCO (-s)
• spotlessCheck passed
• Unit tests passed
• Integration tests passed

Test plan

• Unit test (CalcitePPLChartNullTest) verifying logical plan includes null filter and NULLS LAST
• Integration test (CalciteBinChartNullIT) verifying bin+chart with null values works end-to-end
• YAML REST test (5174.yml) reproducing the exact scenario from the issue

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 2c288d3)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Fix nullable return type for bin bucket UDFs Relevant files: core/src/main/java/org/opensearch/sql/expression/function/udf/binning/MinspanBucketFunction.java core/src/main/java/org/opensearch/sql/expression/function/udf/binning/RangeBucketFunction.java core/src/main/java/org/opensearch/sql/expression/function/udf/binning/SpanBucketFunction.java Sub-PR theme: Add nullsLast sort to chart command and tests Relevant files: core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java integ-test/src/test/java/org/opensearch/sql/calcite/CalciteNoPushdownIT.java ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLChartNullTest.java integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5174.yml Sub-PR theme: Update Claude agent harness documentation Relevant files: .claude/commands/ppl-bugfix.md .claude/harness/ppl-bugfix-followup.md .claude/harness/ppl-bugfix-harness.md .claude/harness/ppl-bugfix-reference.md CLAUDE_GUIDE.md

⚡ Recommended focus areas for review Missing Sort In the early-return branch (when rowSplit == null || columnSplit == null || limit == 0), a nullsLast sort on field(0) was added. Previously there was no sort at all in this branch. It's worth verifying whether this sort is always correct here — for example, when rowSplit == null there may be no meaningful field(0) to sort on, or the sort may be undesirable when limit == 0. relBuilder.sort(relBuilder.nullsLast(relBuilder.field(0))); return relBuilder.peek(); JUnit Version Mismatch The integration test class uses @Test from org.junit.jupiter.api.Test (JUnit 5), but PPLIntegTestCase and the rest of the integration test suite typically use JUnit 4 (org.junit.Test). This may cause the tests to be silently skipped or fail to run depending on the test runner configuration. import org.junit.jupiter.api.Test; import org.opensearch.sql.ppl.PPLIntegTestCase; /** Integration test for GitHub issue #5174: bin/chart NPE with null values. */ public class CalciteBinChartNullIT extends PPLIntegTestCase { @Override public void init() throws Exception { super.init(); enableCalcite(); loadIndex(Index.BANK_WITH_NULL_VALUES); } @Test public void testBinThenChartWithNullValuesShouldNotCauseNPE() throws IOException { Hardcoded SQL The unit test verifies correctness by comparing against a hardcoded expected Spark SQL string. This is brittle — any unrelated refactoring of the SQL generation (e.g., alias naming, whitespace, operator ordering) will break the test even if the logical behavior is correct. Consider verifying the logical plan structure (e.g., presence of IS NOT NULL filter, NULLS LAST in sort) rather than the full SQL string. String expectedSparkSql = "SELECT `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END" + " `category`, SUM(`t2`.`count()`) `count()`\n" + "FROM (SELECT `val_bin`, `category`, COUNT(*) `count()`\n" + "FROM (SELECT `count`, `category`, `subcategory`, `value`," + " SPAN_BUCKET(`value`, 50) `val_bin`\n" + "FROM `scott`.`bounty_numbers`) `t`\n" + "WHERE `val_bin` IS NOT NULL\n" + "GROUP BY `val_bin`, `category`) `t2`\n" + "LEFT JOIN (SELECT `category`, SUM(`count()`) `__grand_total__`, ROW_NUMBER() OVER" + " (ORDER BY SUM(`count()`) DESC) `_row_number_chart_`\n" + "FROM (SELECT `category`, COUNT(*) `count()`\n" + "FROM (SELECT `count`, `category`, `subcategory`, `value`," + " SPAN_BUCKET(`value`, 50) `val_bin`\n" + "FROM `scott`.`bounty_numbers`) `t3`\n" + "WHERE `val_bin` IS NOT NULL\n" + "GROUP BY `val_bin`, `category`) `t7`\n" + "WHERE `category` IS NOT NULL\n" + "GROUP BY `category`) `t10` ON `t2`.`category` = `t10`.`category`\n" + "GROUP BY `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END\n" + "ORDER BY `t2`.`val_bin` NULLS LAST, 2 NULLS LAST"; verifyPPLToSparkSQL(root, expectedSparkSql);

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 2c288d3

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix JUnit version mismatch in test class The test uses @Test from JUnit 5 (org.junit.jupiter.api.Test) but the class extends PPLIntegTestCase, which typically uses JUnit 4. This mismatch means the test methods may not be discovered or executed by the test runner. Change the import to org.junit.Test for JUnit 4 compatibility. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java [16-29] -@Test -public void testBinThenChartWithNullValuesShouldNotCauseNPE() throws IOException { - ... - verifyDataRows( - result, - rows("0-10000", "M", 1), - rows("30000-40000", "F", 1), - rows("30000-40000", "M", 1), - rows("40000-50000", "F", 1)); -} +import org.junit.Test; +// (replace org.junit.jupiter.api.Test with org.junit.Test) Suggestion importance[1-10]: 7 __ Why: The class uses @Test from org.junit.jupiter.api.Test (JUnit 5) but extends PPLIntegTestCase which is a JUnit 4 base class. This mismatch can cause test methods to not be discovered or executed, making the tests ineffective. This is a real correctness issue for the test infrastructure.	Medium
General	Avoid brittle full-SQL string assertions in unit tests The expected SQL string is hardcoded and tightly coupled to internal plan aliases like t2, t10, t3, t7. These aliases are generated by Calcite's planner and may change across versions or refactors, causing brittle test failures unrelated to the actual bug fix. Consider verifying only the structural properties that matter (e.g., presence of NULLS LAST and WHERE val_bin IS NOT NULL) rather than the full SQL string. ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLChartNullTest.java [75-98] -String expectedSparkSql = - "SELECT `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" - + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END" - + " `category`, SUM(`t2`.`count()`) `count()`\n" - ... - + "ORDER BY `t2`.`val_bin` NULLS LAST, 2 NULLS LAST"; -verifyPPLToSparkSQL(root, expectedSparkSql); +String sql = toSparkSQL(root); +assertTrue("Expected NULLS LAST in ORDER BY", sql.contains("NULLS LAST")); +assertTrue("Expected null filter on val_bin", sql.contains("`val_bin` IS NOT NULL")); Suggestion importance[1-10]: 4 __ Why: The suggestion is valid — hardcoded Calcite-generated aliases like t2, t10 are fragile and may break on planner changes. However, the existing pattern of full SQL verification is common in this codebase, and the improved_code changes the testing approach significantly without showing a complete replacement that maintains the same coverage.	Low

---

Previous suggestions

Suggestions up to commit b423fcf

Category	Suggestion	Impact
Possible issue	Fix JUnit version mismatch in test class The test uses @Test from JUnit Jupiter (org.junit.jupiter.api.Test) but the class extends PPLIntegTestCase, which is typically a JUnit 4-based test class. Mixing JUnit 5 annotations with a JUnit 4 base class will cause the test methods to be silently ignored and never executed. Change the import to org.junit.Test to match the JUnit 4 framework used by the base class. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java [16-29] -@Test -public void testBinThenChartWithNullValuesShouldNotCauseNPE() throws IOException { - ... - verifyDataRows( - result, - rows("0-10000", "M", 1), - rows("30000-40000", "F", 1), - rows("30000-40000", "M", 1), - rows("40000-50000", "F", 1)); -} +import org.junit.Test; +// (replace org.junit.jupiter.api.Test with org.junit.Test) Suggestion importance[1-10]: 8 __ Why: The class uses @Test from org.junit.jupiter.api.Test (JUnit 5) but extends PPLIntegTestCase which is a JUnit 4 base class. This mismatch will cause test methods to be silently skipped, making the integration tests non-functional. This is a real bug that would prevent the tests from running.	Medium
General	Avoid brittle full-plan string assertions in tests The test hardcodes a very specific SQL plan string that is tightly coupled to internal Calcite plan generation details (alias names like t2, t10, t7, t3, specific subquery structures). This makes the test extremely brittle — any internal refactoring or Calcite version change will break it even if the fix is correct. Consider verifying only the key behavioral aspects (e.g., presence of NULLS LAST and IS NOT NULL filter) rather than the entire plan string. ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLChartNullTest.java [75-98] -String expectedSparkSql = - "SELECT `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" - + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END" - + " `category`, SUM(`t2`.`count()`) `count()`\n" - ... - + "ORDER BY `t2`.`val_bin` NULLS LAST, 2 NULLS LAST"; -verifyPPLToSparkSQL(root, expectedSparkSql); +String plan = RelOptUtil.toString(root); +assertThat(plan).contains("NULLS LAST"); +assertThat(plan).contains("IS NOT NULL"); Suggestion importance[1-10]: 4 __ Why: The suggestion is valid — hardcoding full Calcite plan strings with internal alias names like t2, t10 makes tests fragile. However, the improved_code uses RelOptUtil and assertThat without showing proper imports or context, and the existing pattern in the codebase may use verifyPPLToSparkSQL consistently, making this a style/maintainability concern rather than a critical issue.	Low

Suggestions up to commit ccb3283

Category	Suggestion	Impact
Possible issue	Verify assumed SQL filter is actually generated The test hardcodes WHERE </code>val_bin<code> IS NOT NULL in the expected SQL, but this filter is not explicitly added in the visitChart fix shown in the diff — the fix only adds NULLS LAST to the sort. If the IS NOT NULL filter is generated by Calcite automatically due to FORCE_NULLABLE + NullPolicy.ANY, this assumption should be verified. If the filter is not actually generated, the expected SQL will not match and the test will fail. ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLChartNullTest.java [75-98] +// Verify the expected SQL matches what Calcite actually generates after the fix. +// The WHERE val_bin IS NOT NULL clause should be confirmed to be emitted by Calcite +// based on NullPolicy.ANY + FORCE_NULLABLE, not assumed. String expectedSparkSql = "SELECT `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END" - ... + + " `category`, SUM(`t2`.`count()`) `count()`\n" + + ... + "ORDER BY `t2`.`val_bin` NULLS LAST, 2 NULLS LAST"; verifyPPLToSparkSQL(root, expectedSparkSql); Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about whether WHERE val_bin IS NOT NULL is actually generated by Calcite due to FORCE_NULLABLE + NullPolicy.ANY, but the improved_code only adds comments without changing the logic. If the filter is not generated, the test would fail, making this a potentially important correctness concern, though it's framed as a verification rather than a concrete fix.	Low
General	Validate test assertions against actual index data The test verifies that only non-null balance rows appear, but the comment says "4 records with balance" while the BANK_WITH_NULL_VALUES index may have a different distribution. The expected rows should be validated against the actual test index data to ensure the assertions are correct and won't produce false positives or false negatives. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java [44-49] +// Verify expected rows match the actual BANK_WITH_NULL_VALUES index data verifyDataRows( result, rows("0-10000", "M", 1), rows("30000-40000", "F", 1), rows("30000-40000", "M", 1), rows("40000-50000", "F", 1)); Suggestion importance[1-10]: 3 __ Why: The suggestion asks to verify that expected rows match actual test index data, but this is a validation concern rather than a code fix. The existing_code and improved_code are essentially identical (only a comment is added), and the test data appears to be derived from the known BANK_WITH_NULL_VALUES index.	Low
	Confirm total reflects result rows not input docs The total field is asserted as 2, but with chart count() over val_bin by category, the result should have rows for each (val_bin, category) combination. With values 10.5 and 20.3 both falling in the 0-50 bin under category "A", and 100.0 in 100-150 under "B", there are 2 distinct groups — this appears correct. However, the total assertion should reflect the number of result rows (2 groups), which matches. Confirm that total in the PPL response refers to the number of result rows and not the number of input documents. integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5174.yml [65-67] +- match: { total: 2 } +- match: { schema: [ { name: val_bin, type: string }, { name: category, type: string }, { name: "count()", type: bigint } ] } +- match: { datarows: [ [ "0-50", "A", 2 ], [ "100-150", "B", 1 ] ] } - Suggestion importance[1-10]: 2 __ Why: The existing_code and improved_code are identical — no actual change is proposed. This is just a verification request about whether total refers to result rows or input documents, which is a low-impact clarification question rather than a concrete fix.	Low

Suggestions up to commit d41e598

Category	Suggestion	Impact
General	Reset transient setting to null instead of false The teardown resets plugins.calcite.enabled to false, but the setup sets it to true as a transient setting. If the test fails mid-run, the teardown may not execute, leaving Calcite enabled for subsequent tests. Consider using a persistent setting or ensuring the teardown is robust, but more critically, the query.settings API call in teardown should be verified to actually reset the state correctly in the test environment. integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5174.yml [48-51] - do: query.settings: body: transient: - plugins.calcite.enabled: false + plugins.calcite.enabled: null Suggestion importance[1-10]: 3 __ Why: Setting plugins.calcite.enabled to null in teardown is a more correct way to clear a transient setting (removing it entirely) rather than setting it to false, which could affect other tests that rely on the default value. This is a minor but valid improvement to test isolation.	Low
	Verify absence of null bin rows in result The verifyDataRows assertion does not account for null bin values that may appear in the result set when NULLS LAST ordering is used. If any null bal_bin rows are returned (e.g., for documents without a balance field), the test will fail unexpectedly. Ensure the test explicitly verifies that null bin rows are absent, or add a filter step in the PPL query to exclude them. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java [44-49] + // Null bin values should be filtered out; verify exactly these 4 non-null rows verifyDataRows( result, rows("0-10000", "M", 1), rows("30000-40000", "F", 1), rows("30000-40000", "M", 1), rows("40000-50000", "F", 1)); + // Also verify total matches expected non-null count + assertEquals(4, result.getJSONArray("datarows").length()); Suggestion importance[1-10]: 2 __ Why: The suggestion adds a redundant assertEquals check that duplicates what verifyDataRows already validates. The PR's core fix (making bucket functions return nullable types and using NULLS LAST sorting) ensures null bin values are filtered out via WHERE val_bin IS NOT NULL in the generated SQL, so the existing assertion is already sufficient.	Low

Suggestions up to commit 1f7053d

Category	Suggestion	Impact
General	Verify test covers null gender combinations The test comment says "4 records with balance" but the BANK_WITH_NULL_VALUES index may have records with null gender values that would appear as null in the result. If any document has a non-null balance but null gender, those rows would be missing from the expected results, causing the test to fail. Verify that all expected rows account for every combination of non-null bal_bin and gender (including null gender) present in the test index. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java [44-49] +// Verify expected rows match all non-null bal_bin combinations including null gender if present verifyDataRows( result, rows("0-10000", "M", 1), rows("30000-40000", "F", 1), rows("30000-40000", "M", 1), rows("40000-50000", "F", 1)); Suggestion importance[1-10]: 3 __ Why: This suggestion asks the developer to verify the test data rather than proposing a concrete code fix. The improved_code is essentially identical to the existing_code with only a comment added, making it a low-impact suggestion.	Low
	Avoid fragile ordinal reference in ORDER BY The expected SQL string uses ordinal 2 in ORDER BY ... 2 NULLS LAST for the second sort key. This is fragile and may break if the query plan changes column ordering. It would be more robust to use the explicit column expression (e.g., the CASE expression alias) instead of a positional reference, or at minimum add a comment explaining why the ordinal is used here. ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLChartNullTest.java [75-98] String expectedSparkSql = "SELECT `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END" - ... - + "ORDER BY `t2`.`val_bin` NULLS LAST, 2 NULLS LAST"; + + " `category`, SUM(`t2`.`count()`) `count()`\n" + + "FROM (SELECT `val_bin`, `category`, COUNT(*) `count()`\n" + + "FROM (SELECT `count`, `category`, `subcategory`, `value`," + + " SPAN_BUCKET(`value`, 50) `val_bin`\n" + + "FROM `scott`.`bounty_numbers`) `t`\n" + + "WHERE `val_bin` IS NOT NULL\n" + + "GROUP BY `val_bin`, `category`) `t2`\n" + + "LEFT JOIN (SELECT `category`, SUM(`count()`) `__grand_total__`, ROW_NUMBER() OVER" + + " (ORDER BY SUM(`count()`) DESC) `_row_number_chart_`\n" + + "FROM (SELECT `category`, COUNT(*) `count()`\n" + + "FROM (SELECT `count`, `category`, `subcategory`, `value`," + + " SPAN_BUCKET(`value`, 50) `val_bin`\n" + + "FROM `scott`.`bounty_numbers`) `t3`\n" + + "WHERE `val_bin` IS NOT NULL\n" + + "GROUP BY `val_bin`, `category`) `t7`\n" + + "WHERE `category` IS NOT NULL\n" + + "GROUP BY `category`) `t10` ON `t2`.`category` = `t10`.`category`\n" + + "GROUP BY `t2`.`val_bin`, CASE WHEN `t2`.`category` IS NULL THEN 'NULL' WHEN" + + " `t10`.`_row_number_chart_` <= 10 THEN `t2`.`category` ELSE 'OTHER' END\n" + // Ordinal 2 refers to the CASE expression (second SELECT column) + + "ORDER BY `t2`.`val_bin` NULLS LAST, `category` NULLS LAST"; verifyPPLToSparkSQL(root, expectedSparkSql); Suggestion importance[1-10]: 2 __ Why: The ordinal 2 in ORDER BY ... 2 NULLS LAST is generated by Calcite's SQL emitter and reflects the actual query plan output. Changing it to \category` NULLS LAST` would cause the test to fail since it wouldn't match the actual generated SQL, making this suggestion potentially incorrect and of low value.	Low

Suggestions up to commit 7edbc20

Category	Suggestion	Impact
General	Fix incorrect comment about expected row count The test verifies that only non-null balance rows appear, but with nullsLast ordering, null bin values would appear at the end rather than being excluded. If the intent is to assert that null bin values are filtered out entirely, the test should also verify the total row count matches exactly 4 (no null rows present). Additionally, the comment says "4 records with balance" but the test data in the YAML fixture only shows 3 non-null value records (IDs 1, 2, 3 have values; ID 4 has null value), so the expected row count in the comment may be incorrect. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinChartNullIT.java [43-49] +// Should only contain rows for non-null balance values (3 records with balance) verifyDataRows( result, rows("0-10000", "M", 1), rows("30000-40000", "F", 1), rows("30000-40000", "M", 1), rows("40000-50000", "F", 1)); Suggestion importance[1-10]: 3 __ Why: The comment says "4 records with balance" but the test data in the YAML fixture shows only 3 non-null value records (IDs 1, 2, 3 have values; ID 4 has null value). However, the verifyDataRows call itself has 4 rows (two different genders for the 30000-40000 range), so the comment is misleading but the actual test data assertions may still be correct depending on the test index data. This is a minor comment fix with low impact.	Low
	Fix incorrect table row count statistic Using 0d as the row count statistic for a table that has 4 rows may cause the query planner to make suboptimal decisions (e.g., choosing wrong join strategies). The row count should reflect the actual number of rows in the test table to produce reliable and deterministic query plans in unit tests. ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLChartNullTest.java [148] -return Statistics.of(0d, ImmutableList.of(), RelCollations.createSingleton(0)); +return Statistics.of(4d, ImmutableList.of(), RelCollations.createSingleton(0)); Suggestion importance[1-10]: 3 __ Why: Using 0d as the row count for a 4-row table could lead to suboptimal query planning decisions. Changing to 4d would better reflect the actual data, though for unit tests focused on logical plan verification rather than execution, this has limited practical impact.	Low

[qianheng-aws]

Decision Log

Root Cause: Bin bucket functions (SpanBucketFunction, MinspanBucketFunction, RangeBucketFunction) declared non-nullable VARCHAR(2000) return type via ReturnTypes.VARCHAR_2000. When input is null, these functions return null, but Calcite's optimizer sees the return type as NOT NULL and removes IS NOT NULL filters (from visitAggregation's nonNullGroupMask) as trivially true. Null group keys then reach TreeMap in Enumerable aggregation, crashing with NPE in compareTo.

Approach:

1. Changed return type to ReturnTypes.VARCHAR_2000.andThen(SqlTypeTransforms.FORCE_NULLABLE) on all three bucket functions — this is the root fix that lets Calcite preserve the null filter
2. Added nullsLast to chart command sort operations as a defensive measure against null keys reaching sort

Alternatives Rejected:

• Adding null checks in the bucket function implementations themselves — treats the symptom, not the root cause; Calcite would still optimize away the filter
• Adding explicit WHERE ... IS NOT NULL in visitChart — redundant if the type is correctly declared nullable, since visitAggregation already adds nonNullGroupMask filters

Pitfalls: The non-nullable return type was inherited from ReturnTypes.VARCHAR_2000 which is Calcite's built-in constant. Easy to miss that UDFs returning null for null inputs need explicit nullable declaration.

Things to Watch: Other UDFs using ReturnTypes.VARCHAR_2000 or similar non-nullable return types that can actually return null — they may have the same latent issue.

[github-actions]

Persistent review updated to latest commit 7edbc20

[github-actions]

Persistent review updated to latest commit 1f7053d

[github-actions]

Persistent review updated to latest commit d41e598

[github-actions]

Persistent review updated to latest commit ccb3283

[LantaoJin]

Is this removing on purpose?

[qianheng-aws]

Yes, #5319 has migrate to reuse the workflow in opensearch-build. The original dedupe workflow has been migrated to that repo for reusing across openseach projects.

[qianheng-aws]

This change is from skill self-improving after receiving the feedback of PR_REVEW workflow comments.

[ahkcs]

Should we add CalciteBinChartNullIT to CalciteNoPushdownIT suite?

[github-actions]

Persistent review updated to latest commit b423fcf

[github-actions]

Persistent review updated to latest commit 2c288d3