You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ConeyLiu (via GitHub)" <gi...@apache.org> on 2023/09/21 02:39:03 UTC
[GitHub] [iceberg] ConeyLiu opened a new pull request, #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
ConeyLiu opened a new pull request, #8605:
URL: https://github.com/apache/iceberg/pull/8605
This ports #8394 to Spark 3.5/3.3/3.2/3.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ConeyLiu commented on a diff in pull request #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on code in PR #8605:
URL: https://github.com/apache/iceberg/pull/8605#discussion_r1332378573
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java:
##########
@@ -699,6 +700,26 @@ public void testDefaultSortOrder() {
assertEquals("Data after compaction should not change", expectedRecords, actualRecords);
}
+ @Test
+ public void testRewriteWithUntranslatedOrUnconvertedFilter() {
+ createTable();
+ Assertions.assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.rewrite_data_files(table => '%s', where => 'lower(c2) = \"fo\"')",
Review Comment:
The where condition has changed to suit the V1 filter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ConeyLiu commented on pull request #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on PR #8605:
URL: https://github.com/apache/iceberg/pull/8605#issuecomment-1728684126
cc @nastra @dramaticlly @advancedxy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ConeyLiu commented on pull request #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on PR #8605:
URL: https://github.com/apache/iceberg/pull/8605#issuecomment-1728914509
Thanks @nastra @advancedxy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ConeyLiu commented on a diff in pull request #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on code in PR #8605:
URL: https://github.com/apache/iceberg/pull/8605#discussion_r1332378810
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java:
##########
@@ -699,6 +700,26 @@ public void testDefaultSortOrder() {
assertEquals("Data after compaction should not change", expectedRecords, actualRecords);
}
+ @Test
+ public void testRewriteWithUntranslatedOrUnconvertedFilter() {
+ createTable();
+ Assertions.assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.rewrite_data_files(table => '%s', where => 'lower(c2) = \"fo\"')",
+ catalogName, tableIdent))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Cannot translate Spark expression");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.rewrite_data_files(table => '%s', where => 'c2 like \"%%fo\"')",
Review Comment:
Same here. Both changed in Spark 3.1/3.2/3.3
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java:
##########
@@ -699,6 +700,26 @@ public void testDefaultSortOrder() {
assertEquals("Data after compaction should not change", expectedRecords, actualRecords);
}
+ @Test
+ public void testRewriteWithUntranslatedOrUnconvertedFilter() {
+ createTable();
+ Assertions.assertThatThrownBy(
+ () ->
+ sql(
+ "CALL %s.system.rewrite_data_files(table => '%s', where => 'lower(c2) = \"fo\"')",
Review Comment:
The where condition has changed to suit the V1 filter. Both changed in Spark 3.1/3.2/3.3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ConeyLiu commented on a diff in pull request #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
Posted by "ConeyLiu (via GitHub)" <gi...@apache.org>.
ConeyLiu commented on code in PR #8605:
URL: https://github.com/apache/iceberg/pull/8605#discussion_r1332382937
##########
spark/v3.3/spark/src/main/scala/org/apache/spark/sql/execution/datasources/SparkExpressionConverter.scala:
##########
@@ -35,7 +35,17 @@ object SparkExpressionConverter {
// Currently, it is a double conversion as we are converting Spark expression to Spark filter
// and then converting Spark filter to Iceberg expression.
// But these two conversions already exist and well tested. So, we are going with this approach.
- SparkFilters.convert(DataSourceStrategy.translateFilter(sparkExpression, supportNestedPredicatePushdown = true).get)
+ DataSourceStrategy.translateFilter(sparkExpression, supportNestedPredicatePushdown = true) match {
Review Comment:
Spark 3.1/3.2/3.3 uses the V1 filters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra merged pull request #8605: Spark 3.1, 3.2, 3.3, 3.5: Throw better exception when filter expression cannot be translated in Rewrite procedure
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra merged PR #8605:
URL: https://github.com/apache/iceberg/pull/8605
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org