Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable predicate pushdown for geography, array and struct in BigQuery #9391

Merged
merged 3 commits into from
Oct 4, 2021

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Sep 27, 2021

  • Run all BigQuery tests (Oct 4)

@ebyhr ebyhr force-pushed the ebi/bigquery-disable-pushdown branch from 5de6f5f to 127cab4 Compare September 27, 2021 03:52
@cla-bot cla-bot bot added the cla-signed label Sep 27, 2021
@ebyhr
Copy link
Member Author

ebyhr commented Sep 27, 2021

CI failure in Phoenix looks #9199 though it's different test method.

@ebyhr ebyhr force-pushed the ebi/bigquery-disable-pushdown branch from c06b146 to 2a63eda Compare October 1, 2021 02:05
Comment on lines 315 to 323
if (this == GEOGRAPHY || type instanceof ArrayType || type instanceof RowType) {
return Optional.empty();
Copy link
Member

Choose a reason for hiding this comment

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

  • what is the enum value for Array and Row. Is it RECORD?
  • i would let inidivudla enum option's toStringconverter handle the logic, by returning Optional

Copy link
Member

Choose a reason for hiding this comment

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

i would let inidivudla enum option's toStringconverter handle the logic, by returning Optional

i pushed a fixup with some proposed changes, but up to you whether you take them or discard

Copy link
Member Author

Choose a reason for hiding this comment

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

what is the enum value for Array and Row. Is it RECORD?

Row is RECORD, but field type is used in case of Array. e.g. Array<boolean> -> BOOLEAN.
So, we can't remove the if (type instanceof ArrayType) until fixing it.

@ebyhr ebyhr force-pushed the ebi/bigquery-disable-pushdown branch from 6689bd5 to b752f85 Compare October 4, 2021 03:05
@ebyhr ebyhr merged commit caf1417 into trinodb:master Oct 4, 2021
@ebyhr ebyhr deleted the ebi/bigquery-disable-pushdown branch October 4, 2021 04:24
@github-actions github-actions bot added this to the 363 milestone Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants