You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "karuppayya (via GitHub)" <gi...@apache.org> on 2023/04/19 22:43:44 UTC
[GitHub] [iceberg] karuppayya opened a new pull request, #7380: Cherry pick Order case sensitivity changes to 2.4
karuppayya opened a new pull request, #7380:
URL: https://github.com/apache/iceberg/pull/7380
CP-ed commit https://github.com/apache/iceberg/commit/6504fd809199cbc88ef890cc2f5b42a2cf00e102
--
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] RussellSpitzer commented on a diff in pull request #7380: Cherry pick Order case sensitivity changes to 3.4
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #7380:
URL: https://github.com/apache/iceberg/pull/7380#discussion_r1174035778
##########
spark/v3.4/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSetWriteDistributionAndOrdering.java:
##########
@@ -64,6 +67,28 @@ public void testSetWriteOrderByColumn() {
Assert.assertEquals("Should have expected order", expected, table.sortOrder());
}
+ @Test
+ public void testSetWriteOrderWithCaseSensitiveColumnNames() {
+ sql(
+ "CREATE TABLE %s (Id bigint NOT NULL, Category string, ts timestamp, data string) USING iceberg",
+ tableName);
+ Table table = validationCatalog.loadTable(tableIdent);
+ Assert.assertTrue("Table should start unsorted", table.sortOrder().isUnsorted());
+ sql("SET %s=true", SQLConf.CASE_SENSITIVE().key());
+ Assertions.assertThatThrownBy(
+ () -> {
+ sql("ALTER TABLE %s WRITE ORDERED BY category, id", tableName);
+ })
+ .isInstanceOf(ValidationException.class);
+
+ sql("SET %s=false", SQLConf.CASE_SENSITIVE().key());
+ Assertions.assertThatNoException()
+ .isThrownBy(
+ () -> {
+ sql("ALTER TABLE %s WRITE ORDERED BY category, id", tableName);
+ });
Review Comment:
I would prefer we just run the Alter and then check that the sortOrder is correctly set. Personally, I'm generally not a fan of "did this not throw an exception" tests.
--
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] karuppayya commented on pull request #7380: Cherry pick Order case sensitivity changes to 2.4
Posted by "karuppayya (via GitHub)" <gi...@apache.org>.
karuppayya commented on PR #7380:
URL: https://github.com/apache/iceberg/pull/7380#issuecomment-1515474656
@RussellSpitzer @aokolnychyi
--
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] RussellSpitzer commented on pull request #7380: Cherry pick Order case sensitivity changes to 3.4
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #7380:
URL: https://github.com/apache/iceberg/pull/7380#issuecomment-1518158267
Weird Project Nessie Dependency failure? Maybe just need to rebase to master?
--
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] RussellSpitzer commented on a diff in pull request #7380: Cherry pick Order case sensitivity changes to 3.4
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #7380:
URL: https://github.com/apache/iceberg/pull/7380#discussion_r1174036171
##########
spark/v3.4/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSetWriteDistributionAndOrdering.java:
##########
@@ -64,6 +67,28 @@ public void testSetWriteOrderByColumn() {
Assert.assertEquals("Should have expected order", expected, table.sortOrder());
}
+ @Test
+ public void testSetWriteOrderWithCaseSensitiveColumnNames() {
+ sql(
+ "CREATE TABLE %s (Id bigint NOT NULL, Category string, ts timestamp, data string) USING iceberg",
+ tableName);
+ Table table = validationCatalog.loadTable(tableIdent);
+ Assert.assertTrue("Table should start unsorted", table.sortOrder().isUnsorted());
+ sql("SET %s=true", SQLConf.CASE_SENSITIVE().key());
+ Assertions.assertThatThrownBy(
Review Comment:
Can we add a message check to this, just want to make sure the validation exception we get is related to the names not matching
--
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] RussellSpitzer merged pull request #7380: Cherry pick Order case sensitivity changes to 3.4
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer merged PR #7380:
URL: https://github.com/apache/iceberg/pull/7380
--
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