You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2025/03/09 22:48:05 UTC
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Csaba Ringhofer has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22603
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M be/src/exprs/expr-codegen-test.cc
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
10 files changed, 101 insertions(+), 44 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/1
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 13:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22603/12/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/12/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS12, Line 37: return arr;
> The function was removed, so this comment is no longer relevant. The same c
Ack
http://gerrit.cloudera.org:8080/#/c/22603/13/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/13/fe/src/main/java/org/apache/impala/util/StringUtils.java@50
PS13, Line 50: Preconditions.checkState(canFail, "UTF8 decoding failed.");
How about adding the error message of 'ex'? It might explain why it fails.
Preconditions.checkState(canFail, "UTF8 decoding failed: %s", ex.getMessage());
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 05:55:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 15
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 12:23:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 10:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG@42
PS9, Line 42: large polygons. A follow up commit may refine this, e.g. by
> polyigons?
Done
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG@50
PS9, Line 50: - Added a few tests that push down non-ascii const expressions in
> nit: double space
Done
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> This can show up in CatalogObjects.TColumn. That seems like it would make i
>This can show up in CatalogObjects.TColumn.
Another case is THdfsPartition.partitionKeyExprs, which is used very frequently.
Btw actually in binary/compact encoding the string and binary type are encoded exactly the same way.
https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 30 May 2025 09:19:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 3: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11603/
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 15:18:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 2: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11586/
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Mar 2025 21:13:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 13:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19581/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 13:41:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#12).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polygons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
18 files changed, 357 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/12
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 12
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11603/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 09:57:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 5:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/5/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/5/common/thrift/Exprs.thrift@120
PS5, Line 120: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 20:30:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18573/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 20:32:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11704/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 07 Apr 2025 17:06:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 13:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/13/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/13/common/thrift/Exprs.thrift@121
PS13, Line 121: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 13:16:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#9).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polyigons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
15 files changed, 326 insertions(+), 61 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/9
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19309/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 26 May 2025 13:53:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19310/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 26 May 2025 14:03:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18524/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Mar 2025 15:26:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@69
PS5, Line 69: strValue_ = value;
> Maybe we could keep the byte array, and generate the String when needed? Th
Convinced myself that always keeping byte array is too risky as the frequent utf8 decoding may cause perf regression.
Changed the code to only keep binValue_ if utf8 decoding is not successful and keep strValue_ otherwise.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 07 Apr 2025 17:08:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19546/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Jun 2025 18:48:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 6:
(3 comments)
The change is still WIP, I didn't want to finalize things before agreeing on storing with String vs byte[]
>Supprised that we don't need BE changes.
Thrift's binary and string are both std::string in c++, so luckily no change was needed
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@69
PS5, Line 69: binValue_ = StringUtils.toUtf8Array(value);
> Can we just generate this byte array in need? StringLiteral is also used in
Maybe we could keep the byte array, and generate the String when needed? This could decrease the memory footprint, as in most case 1 byte per char would be enough instead of 2. If I see correctly we keep the strings in unescaped format.
https://github.com/apache/impala/blob/32836eab4a42541e44d95435f815a5d0cf9d3b24/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java#L1280
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/util/StringUtils.java@28
PS5, Line 28: class St
> Don't need "abstract".
Done
http://gerrit.cloudera.org:8080/#/c/22603/5/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:
http://gerrit.cloudera.org:8080/#/c/22603/5/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@488
PS5, Line 488: concat("a", "á")
> Can we add tests for UTF-8 aware string functions in both UTF-8 and non-UTF
done, added a test that use a few utf8 aware functions
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 05 Apr 2025 10:40:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Mar 2025 01:57:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 4:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/4/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/4/common/thrift/Exprs.thrift@120
PS4, Line 120: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 20:09:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11586/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Mar 2025 15:05:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 3:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/3/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/3/common/thrift/Exprs.thrift@120
PS3, Line 120: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 09:58:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 14:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/14/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/14/common/thrift/Exprs.thrift@121
PS14, Line 121: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 11:04:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS10, Line 37: Preconditions.checkState(false, "UTF8 encoding failed.");
: return null;
> Added messages to the preconditions.
I see. Can we avoid creating an encoder each time? Using the following code, we don't need to handle exceptions.
return StandardCharsets.UTF_8.encode(CharBuffer.wrap(str));
http://gerrit.cloudera.org:8080/#/c/22603/12/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/12/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS12, Line 37: Preconditions.checkState(false, "UTF8 encoding failed.");
nit: If we do need this, how about adding the string in the error message?
Preconditions.checkState(false, "UTF8 encoding failed for %s", str);
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 12
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 09:18:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 15: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 15
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 18:22:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11585/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 09 Mar 2025 22:48:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 6:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/6/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/6/common/thrift/Exprs.thrift@120
PS6, Line 120: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 05 Apr 2025 10:32:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 7:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/7/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/7/common/thrift/Exprs.thrift@120
PS7, Line 120: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 07 Apr 2025 17:07:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 8:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/8/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/8/common/thrift/Exprs.thrift@121
PS8, Line 121: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 26 May 2025 13:28:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#11).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polygons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
18 files changed, 357 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/11
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#14).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polygons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
18 files changed, 345 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/14
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 13:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS10, Line 37: return arr;
: }
> I see. Can we avoid creating an encoder each time? Using the following code
Convinced myself that java String->UTF8 conversion should always succeed, so this function is no longer necessary, using StandardCharsets.UTF_8.encode() directly is faster and simpler.
http://gerrit.cloudera.org:8080/#/c/22603/12/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/12/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS12, Line 37: return arr;
> nit: If we do need this, how about adding the string in the error message?
The function was removed, so this comment is no longer relevant. The same could be applied to fromUtf8Buffer, but I am not sure there, would it we useful to show the string, e.g. in hex?
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 13:18:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22603/13/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/13/fe/src/main/java/org/apache/impala/util/StringUtils.java@50
PS13, Line 50: Preconditions.checkState(canFail, "UTF8 decoding failed: %s", ex.getMessage());
> How about adding the error message of 'ex'? It might explain why it fails.
Done
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 11:02:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#2).
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
12 files changed, 244 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/2
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 26 May 2025 20:06:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 11:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> Before PS11 non-UTF8 default values hit a precondition. Added checks for th
Ack
http://gerrit.cloudera.org:8080/#/c/22603/11/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/11/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@71
PS11, Line 71: I
nit: "It" ?
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS10, Line 37: Preconditions.checkState(false);
: return null;
> The goal is to throw exception, the return null is just for the compiler.
Can we throw the exception explicitly with the string in the message? Using checkState(false) we will only see "IllegalStateException: null" if it fails, which isn't friendly for debug.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 21 Jun 2025 00:49:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 10:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> Ack
As Michael mentioned, TStringLiteral is used in TColumn which has a 'default_value' field that is Exprs.TExpr. I see that 'default_value' is used in KuduCatalogOpExecutor in catalogd and the value must be sent from coordinators.
How about creating a kudu table with default column values and altering the default value of a kudu column? If the default value has non-UTF8 characters, will that break the compatibility? If so, we'd better increase the CatalogServiceVersion in CatalogService.thrift.
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@303
PS10, Line 303: if (getClass() != other.getClass()) return -1; // TODO: is this a valid case?
This looks weird. Do we have a JIRA to further investigate this?
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@24
PS10, Line 24: import java.nio.charset.CharacterCodingException;
: import java.nio.charset.CoderResult;
nit: not used
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@27
PS10, Line 27: import java.util.List;
: import java.util.ArrayList;
: import java.util.stream.Collectors;
nit: not used
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@76
PS10, Line 76: Is is ok the
nit: "It is ok to" ?
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@148
PS10, Line 148: //Preconditions.checkState(needsUnescaping_);
Will this fail?
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@209
PS10, Line 209: hexa
nit: "hex"
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@247
PS10, Line 247: Preconditions.checkState(strValue_ != null);
Can we add error messages to such new Preconditions.checkState() call sites? So when it fails, we know which part is wrong in a long query. E.g.
Preconditions.checkState(strValue_ != null, "non-utf8 string: %s", getNormalizedValue());
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@299
PS10, Line 299:
nit: has an extra space
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS10, Line 37: Preconditions.checkState(false);
: return null;
Do we want to return null or throw an IllegalStateException by checkState() here?
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Jun 2025 10:07:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> >This can show up in CatalogObjects.TColumn.
So the message format is unchanged, that's good. Sending an update with non-UTF8 characters to an older catalogd would result in an error? But don't see a way around that.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 30 May 2025 17:23:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG@42
PS9, Line 42: large polyigons. A follow up commit may refine this, e.g. by
polyigons?
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG@50
PS9, Line 50: - Added a few tests that push down non-ascii const expressions in
nit: double space
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> Changing type of field 'value' from string to binary in TStringLiteral migh
This can show up in CatalogObjects.TColumn. That seems like it would make it a breaking change for zero-downtime upgrades.
I can't think of a good way to implement this that works with both older and newer catalogd unless we know what version of catalogd we're talking to though. Maybe a 2nd field, then populate "string value" if UTF-8, "binary bytes" if not? Most likely that would fail the catalog operation if we tried to send a TStringLiteral with only bytes filled in to an older catalogd, but that could be an ok failure mode.
I think we require that coordinator and executors are all the same version, so the Java <-> C++ translation (and possibility of crashes) wouldn't be a concern.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 29 May 2025 21:45:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> yeah, it would lead to an error, but it probably also leads to an error aft
Ack
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 02 Jun 2025 21:20:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#13).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polygons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
18 files changed, 345 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/13
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 13
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 14:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19599/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 11:28:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> So the message format is unchanged, that's good. Sending an update with non
yeah, it would lead to an error, but it probably also leads to an error after the patch, for example if try to do anything with such a value which probably converts it to a String which will fail
creating partitions with non-UTF8 strings seems to be buggy (IMPALA-14096), but the problems start before sending anything to catalogd. I am also not convinced that Hive works as intended in this case, I didn't have time to dig into it.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 30 May 2025 18:03:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19579/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 12
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 08:14:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 11:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
> As Michael mentioned, TStringLiteral is used in TColumn which has a 'defaul
Before PS11 non-UTF8 default values hit a precondition. Added checks for this in the analyzer to get better error message and added a few tests.
Non-utf8 defaults could make sense for BINARY columns, but found out that those don't work anyway, created ticket IMPALA-14173
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@303
PS10, Line 303: // Sublcass implementations do not handle comparison with other Literal types.
> This looks weird. Do we have a JIRA to further investigate this?
Changed it to Precondition, if no test hits it, then I think that it is ok like this, the way it worked really didn't make sense.
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@24
PS10, Line 24: import java.util.Arrays;
:
> nit: not used
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@27
PS10, Line 27: import org.apache.impala.catalog.ScalarType;
: import org.apache.impala.catalog.Type;
: import org.apache.impala.catalog.Ty
> nit: not used
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@76
PS10, Line 76: inValue_ = v
> nit: "It is ok to" ?
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@148
PS10, Line 148:
> Will this fail?
Yes, it failed in some tests, forgot to remove it completely.
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@209
PS10, Line 209:
> nit: "hex"
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@247
PS10, Line 247: // We allow simple chaining of MINUS to recognize negative numbers.
> Can we add error messages to such new Preconditions.checkState() call sites
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@299
PS10, Line 299:
> nit: has an extra space
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS10, Line 37: Preconditions.checkState(false);
: return null;
> Do we want to return null or throw an IllegalStateException by checkState()
The goal is to throw exception, the return null is just for the compiler.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Jun 2025 18:45:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 11:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/11/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/11/common/thrift/Exprs.thrift@121
PS11, Line 121: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 11
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Jun 2025 18:20:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#8).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polyigons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
15 files changed, 326 insertions(+), 61 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/8
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 8
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 07 Apr 2025 22:28:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18523/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 09 Mar 2025 23:13:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 2:
(2 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/2/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/2/common/thrift/Exprs.thrift@120
PS2, Line 120: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
http://gerrit.cloudera.org:8080/#/c/22603/2/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/2/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@125
PS2, Line 125: String redecoded = StandardCharsets.UTF_8.newDecoder().decode(msg.string_literal.value.duplicate()).toString();
line too long (118 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Mar 2025 15:01:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 15:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12083/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 15
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 12:23:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polygons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Reviewed-on: http://gerrit.cloudera.org:8080/22603
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/ColumnDef.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
18 files changed, 345 insertions(+), 63 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 16
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 9:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/9/common/thrift/Exprs.thrift@121
PS9, Line 121: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 26 May 2025 13:37:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 10: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG@42
PS9, Line 42: large polygons. A follow up commit may refine this, e.g. by
> Done
Ack
http://gerrit.cloudera.org:8080/#/c/22603/9//COMMIT_MSG@50
PS9, Line 50: - Added a few tests that push down non-ascii const expressions in
> Done
Ack
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 30 May 2025 17:24:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#4).
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
13 files changed, 229 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/4
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 5:
(3 comments)
This will be quite helpful! Supprised that we don't need BE changes.
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@69
PS5, Line 69: binValue_ = StringUtils.toUtf8Array(value);
Can we just generate this byte array in need? StringLiteral is also used in partition values cached in the catalog. I think we don't need the byte arrays there. It increases the metadata footprint.
https://github.com/apache/impala/blob/32836eab4a42541e44d95435f815a5d0cf9d3b24/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java#L174
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/5/fe/src/main/java/org/apache/impala/util/StringUtils.java@28
PS5, Line 28: abstract
Don't need "abstract".
http://gerrit.cloudera.org:8080/#/c/22603/5/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:
http://gerrit.cloudera.org:8080/#/c/22603/5/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@488
PS5, Line 488: concat("a", "á")
Can we add tests for UTF-8 aware string functions in both UTF-8 and non-UTF-8 modes? E.g. length(concat()).
We need to make sure the resutls like length are consistent with BE.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 24 Mar 2025 03:29:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18567/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 10:24:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 1:
(6 comments)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/1/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/1/common/thrift/Exprs.thrift@119
PS1, Line 119: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
File fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java:
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java@199
PS1, Line 199: * Returns null for types that do not have a LiteralExpr subclass,
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@111
PS1, Line 111: byte[] val = (needsUnescaping_) ? StandardCharsets.UTF_8.encode(getUnescapedValue()).array() : binValue_;
line too long (110 > 90)
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@259
PS1, Line 259: if (binValue_[i] < other.binValue_[i]) return 1;
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
File fe/src/test/java/org/apache/impala/service/JniFrontendTest.java:
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@138
PS1, Line 138: TStringLiteral secretKey = new TStringLiteral(StandardCharsets.UTF_8.encode("openai-api-key-secret"));
line too long (106 > 90)
http://gerrit.cloudera.org:8080/#/c/22603/1/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@143
PS1, Line 143:
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 09 Mar 2025 22:50:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18574/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 20:55:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18843/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 05 Apr 2025 10:57:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#10).
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
IMPALA-10349: Support constant folding for non ascii strings
Before this patch constant folding only converted the result of an
expression to StringLiteral if all characters were ASCII. The
change allows both UTF8 strings with non ascii characters and
byte arrays that are not valid UTF8 strings - the latter can
occur when constant folding is applied to BINARY columns,
for example in geospatial functions like st_polygon().
The main goal is being able to push down more predicates, e.g.
before that patch a filter like col="á" couldn't be pushed down
to Iceberg/Kudu/Parquet stat filtering, as all these expect literals.
Main changes:
- TStringLiteral uses a binary instead of a string member.
This doesn't affect BE as in c++ both types are compiled
to std::string. In Jave a java.nio.ByteBuffer is used instead of
String.
- StringLiteral uses a byte[] member to store the value of
the literal in case it is not valid UTF8 and cannot be
represented as Java String. In other cases still a String
is used to keep the change minimal, though it may be more
optimal to use UTF8 byte[] due to the smaller size. Always
converting from byte[] to String may be costy in the catalog
as partition values are stored as *Literals and rest of the
catalog operates on String.
- StringLiteral#compareTo() is switched to byte wise compare on byte[]
to be consistent with BE. This was not needed for ASCII strings
as Java String behaves the same way in that case, but non-ASCII
can have different order (note that Impala does not support
collations).
- When an invalid UTF8 StringLiteral is printed, for example in
case of EXPLAIN output, then it is printed as
unhex("<byte array in hexadecimal>"). This is a non-lossy way to
represent it, but it may be too verbose in some cases, e.g. for
large polygons. A follow up commit may refine this, e.g. by
limiting the max size printed.
An issue found while implementing this is that INSERT does not
handle invalid UTF8 partition values correctly, see IMPALA-14096.
This behavior is not changed in the patch.
Testing:
- Added a few tests that push down non-ascii const expressions in
predicates (both with utf8_mode=true and false).
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
15 files changed, 326 insertions(+), 61 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/10
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 10
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 12:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22603/12/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:
http://gerrit.cloudera.org:8080/#/c/22603/12/common/thrift/Exprs.thrift@121
PS12, Line 121: 1: required binary value;
Changing type of field 'value' from string to binary in TStringLiteral might break the compatibility between impalad and catalogd/statestore during upgrade
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 12
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 07:49:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22603/11/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
File fe/src/main/java/org/apache/impala/analysis/StringLiteral.java:
http://gerrit.cloudera.org:8080/#/c/22603/11/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java@71
PS11, Line 71: I
> nit: "It" ?
Done
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java
File fe/src/main/java/org/apache/impala/util/StringUtils.java:
http://gerrit.cloudera.org:8080/#/c/22603/10/fe/src/main/java/org/apache/impala/util/StringUtils.java@37
PS10, Line 37: Preconditions.checkState(false, "UTF8 encoding failed.");
: return null;
> Can we throw the exception explicitly with the string in the message? Using
Added messages to the preconditions.
I think that using preconditions is better here than throwing another exception, as I consider these program errors. Java String->UTF8 should always succeed AFAIK, fromUtf8Buffer() with canFail==true is only called where we shouldn't see bad strings (Kudu default values/range partitions come from string literals that are checked for validity). One thing that could introduce invalid strings is an existing Kudu table with range partitions on a binary column that is not valid utf8. Currently you can't create such tables in Impala, so I couldn't check how we handle these:
create table tkudubinpart (b binary primary key, i int) partition by range(b) (partition values < cast("a" as binary)) stored as kudu;
result:
ImpalaRuntimeException: Error creating Kudu table 'impala::default.tkudubinpart'
CAUSED BY: ImpalaRuntimeException: Key columns not supported for type: Type: binary
I can create a ticket for adding support for range partitions on binary columns, but I am not sure if this is really an important use case.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 12
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 24 Jun 2025 08:30:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11606/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Mar 2025 20:28:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#3).
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
13 files changed, 229 insertions(+), 50 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/3
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11935/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 9
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 26 May 2025 13:42:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10349: Support constant folding for non ascii strings
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 14: Code-Review+2
LGTM. Carrying Michael's +1. Thanks for adding this!
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 25 Jun 2025 11:58:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#6).
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
15 files changed, 286 insertions(+), 54 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/6
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 6
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#5).
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
13 files changed, 229 insertions(+), 53 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/5
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 5
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22603
to look at the new patch set (#7).
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
WIP IMPALA-10349: Support constant folding for non ascii strings
Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
---
M common/thrift/Exprs.thrift
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/main/java/org/apache/impala/util/KuduUtil.java
A fe/src/main/java/org/apache/impala/util/StringUtils.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/constant-folding-utf8-mode.test
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
M testdata/workloads/functional-query/queries/QueryTest/binary-type.test
15 files changed, 315 insertions(+), 55 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/22603/7
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18851/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 7
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 07 Apr 2025 17:32:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-10349: Support constant folding for non ascii strings
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22603 )
Change subject: WIP IMPALA-10349: Support constant folding for non ascii strings
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/11585/
--
To view, visit http://gerrit.cloudera.org:8080/22603
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70663457a0b0a3443e586350f0a5996bb75ba64a
Gerrit-Change-Number: 22603
Gerrit-PatchSet: 1
Gerrit-Owner: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Mar 2025 04:08:35 +0000
Gerrit-HasComments: No