JSON predicate pushdown for Pinot

[robertzych]

Description

These changes allow the Trino-Pinot connector to perform predicate pushdowns for a combination of JSON functions.

For example, the following Trino function calls:

where
    contains(ARRAY['O'], json_extract_scalar(json, '$.orderstatus')) 
    AND (
      json_array_contains(json_extract(json, '$.stringArray'), 'O') 
      OR json_array_contains(json_extract(json, '$.stringArray'), 'F') 
    ) 
    AND (
        json_extract(json, '$.dne') IS NULL 
        OR json_array_contains(json_extract(json, '$.stringArray'), 'X') = false
        OR json_array_contains(json_extract(json, '$.intArray'), 0)
    );

Are translated to the following Pinot function calls:

where (
    JSON_MATCH(json, '"$.orderstatus" in (''O'')') 
    AND (
        JSON_MATCH(json, '"$.stringArray[*]" = ''O''') 
        OR JSON_MATCH(json, '"$.stringArray[*]" = ''F''')
    ) 
    AND (
        JSON_MATCH(json, '"$.dne" IS NULL') 
        OR NOT(JSON_MATCH(json, '"$.stringArray[*]" = ''X'''))
        OR JSON_MATCH(json, '"$.intArray[*]" = 0')
    )
)

The new session property (pinot.json_predicate_pushdown_enabled) defaults to false because JSON_MATCH can fail when a JSON index hasn't been configured in Pinot.

TODO: documentation for the supported JSON predicates and new session property

Additional context and related issues

PinotMetadata.applyFilter calls buildConstraintPQL which recursively walks through the call tree and returns the converted SQL if the entire call tree is supported.

Release notes

TODO

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(X) Release notes are required, with the following suggested text:
TODO

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Robert Zych.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[robertzych]

@cla-bot check

[robertzych]

I just signed and submitted the individual CLA.

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[cla-bot]

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

[robertzych]

@cla-bot check

[ebyhr]

Please add query based tests to TestPinotConnectorTest or TestPinotConnectorSmokeTest. assertThat(query("...")) provides isFullyPushedDown() method.

[ebyhr]

Why do we need this toggle?

[ebyhr]

The new session property (pinot.json_predicate_pushdown_enabled) defaults to false because JSON_MATCH can fail when a JSON index hasn't been configured in Pinot.

Is it possible to check the index config during analyze?

[robertzych]

There isn't a way to determine if the JSON index has been completely applied. Retrieving the table config from the Pinot Controller API (/tables/{tableName}) and inspecting tableIndexConfig.jsonIndexColumns would only confirm that it has been configured and the query could still fail if the JSON index was recently added and the table's segments haven't been reloaded.

[robertzych]

@ebyhr Assuming a missing JSON index rarely happens (or only during development), I propose we change the default value of this config to true. Do you agree?

[mosabua]

I think we definitely need this toogle. If we can switch it to true by default is a bit interesting. @robertzych mentioned in the Trino Contributor Call https://github.com/trinodb/trino/wiki/Contributor-meetings#trino-contributor-call-13-dec-2024 that this could lead to a scenario where queries that used to work fail after an upgrade to a Trino version with this pushdown support.

So I think it depends a bit on what we did in similar situations for other connectors. Given that it can be breaking queries for users we seem to need the toggle.

[mosabua]

I am hoping that @ebyhr or @elonazoulay or maybe others can help review the code. At a minimum we need to document the new flag in the properties docs section for the connector and add some info about this pushdown in the performance section.

[elonazoulay]

This is a great feature! I'll take another pass, left some nits.