Skip to content

Conversation

@LantaoJin
Copy link
Member

@LantaoJin LantaoJin commented Oct 29, 2025

Description

  • usenull=<bool>: optional (since 3.4.0). whether to output the null value. The default value of usenull is determined by plugins.ppl.syntax.legacy.preferred:
    • When plugins.ppl.syntax.legacy.preferred=true, usenull defaults to true
    • When plugins.ppl.syntax.legacy.preferred=false, usenull defaults to false

See #4684 for examples.

Related Issues

Resolves #4684

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@LantaoJin LantaoJin added the enhancement New feature or request label Oct 29, 2025
Signed-off-by: Lantao Jin <[email protected]>
Comment on lines +31 to +32
* When ``plugins.ppl.syntax.legacy.preferred=true``, ``usenull`` defaults to ``true``
* When ``plugins.ppl.syntax.legacy.preferred=false``, ``usenull`` defaults to ``false``
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bullet points in blockquote, is it expected?

Copy link
Member Author

@LantaoJin LantaoJin Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Keep the format same with the current stats.rst. I tried to use the new format to align with Update PPL Command Documentation. But seems there are still many format problems in that PR. ref

@qianheng-aws qianheng-aws merged commit c6a5fb9 into opensearch-project:main Oct 30, 2025
35 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.19-dev failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-4696-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c6a5fb9762aed964fe56513b80a55f35934b0255
# Push it to GitHub
git push --set-upstream origin backport/backport-4696-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-dev

Then, create a pull request where the base branch is 2.19-dev and the compare/head branch is backport/backport-4696-to-2.19-dev.

Comment on lines +1875 to +1879
context.relBuilder.filter(
PlanUtils.getSelectColumns(groupByList).stream()
.map(context.relBuilder::field)
.map(context.relBuilder::isNotNull)
.toList());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not apply isNotNull directly on groupByList, but on their underlying input ref? If some operation converts a null field to a non-null one, I think it should not be filtered out.

Copy link
Member Author

@LantaoJin LantaoJin Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuancu
Think about query:

| eval a = nullif(status, "200") | stats bucket_nullable = false count() by a

The groupByList contains RexCall "AS($9, 'a')". So we finally build a
context.relBuilder.filter(isNotNull($9)) which $9 is nullif(status, "200") instead of status

LantaoJin added a commit to LantaoJin/search-plugins-sql that referenced this pull request Oct 30, 2025
…-project#4696)

* Support 'usenull' option in PPL top and rare commands

Signed-off-by: Lantao Jin <[email protected]>

* fix the incorrect naming

Signed-off-by: Lantao Jin <[email protected]>

---------

Signed-off-by: Lantao Jin <[email protected]>
(cherry picked from commit c6a5fb9)
@LantaoJin LantaoJin added the backport-manually Filed a PR to backport manually. label Oct 30, 2025
LantaoJin added a commit that referenced this pull request Oct 30, 2025
…commands (#4696) (#4702)

* Support 'usenull' option in PPL `top` and `rare` commands (#4696)

* Support 'usenull' option in PPL top and rare commands

Signed-off-by: Lantao Jin <[email protected]>

* fix the incorrect naming

Signed-off-by: Lantao Jin <[email protected]>

---------

Signed-off-by: Lantao Jin <[email protected]>
(cherry picked from commit c6a5fb9)

* fix compile error

Signed-off-by: Lantao Jin <[email protected]>

* fix compile error

Signed-off-by: Lantao Jin <[email protected]>

---------

Signed-off-by: Lantao Jin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19-dev backport-failed backport-manually Filed a PR to backport manually. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] The top rare command should not return null

4 participants