You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/09/26 15:40:53 UTC
[GitHub] [iceberg] singhpk234 opened a new pull request, #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
singhpk234 opened a new pull request, #5860:
URL: https://github.com/apache/iceberg/pull/5860
### About the change
Solves https://github.com/apache/iceberg/issues/5104
Presently when `spark.sql.datetime.java8API.enabled` is set to true,
- java.time.LocalDate is returned for Spark SQL DATE type
- java.time.Instant is returned for Spark SQL TIMESTAMP type
so always casting date type to java.sql.Date or timestamp to java.sql.Timestamp is not correct and lead to class cast exception. This change attempts to use appropriate api's from DateTimeUtils class which take these scenarios into consideration and calls the relevant api's as per the object type.
```scala
/**
* Converts an Java object to days.
*
* @param obj Either an object of `java.sql.Date` or `java.time.LocalDate`.
* @return The number of days since 1970-01-01.
*/
def anyToDays(obj: Any): Int = obj match {
case d: Date => fromJavaDate(d)
case ld: LocalDate => localDateToDays(ld)
}
/**
* Converts an Java object to microseconds.
*
* @param obj Either an object of `java.sql.Timestamp` or `java.time.Instant`.
* @return The number of micros since the epoch.
*/
def anyToMicros(obj: Any): Long = obj match {
case t: Timestamp => fromJavaTimestamp(t)
case i: Instant => instantToMicros(i)
}
```
----
### Testing Done
Added an UT which would fail without this change.
cc @rdblue @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] aokolnychyi commented on pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#issuecomment-1314549029
Thanks, @singhpk234! Could you, please, cherry-pick these changes to 3.2 and 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] singhpk234 commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r981608257
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +90,57 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDatePartitionedTables() {
Review Comment:
added an UT for timestamp as well.
--
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] singhpk234 commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r1018512446
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
Review Comment:
removed the ut's for java8 date/ time disabled, and seperated out timestamp & date partitioned table UT into diff methods.
--
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] singhpk234 commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r981970743
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
+ for (String scenario : scenarios) {
Review Comment:
> Instead of these two for loops, we can use Junit's ParameterizedTest with CsvSource, it will be very clean compared to this. (as we already depend on Junit5)
+1 on the suggestion
I tried making this UT a parameterized test as per last feedback, but this UT didn't ran when i ran the complete suite, I think this is happening due to `SparkCatalogTestBase` already running with a parameterized class and we extending that.
https://github.com/apache/iceberg/blob/a8b67085e671bb5fa5a7f738ce2e9cb2dcc65d9c/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/SparkCatalogTestBase.java#L27
Let me dig in more.
--
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] aokolnychyi merged pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
aokolnychyi merged PR #5860:
URL: https://github.com/apache/iceberg/pull/5860
--
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] singhpk234 commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r997874540
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
+ for (String scenario : scenarios) {
Review Comment:
I read though the JUnit's code since, to deep dive on observed behaviour, we are using Parametrized class via SparkCatalogTestBase which by default uses [BlockJUnit4ClassRunnerWithParametersFactory](https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/runners/Parameterized.java#L270) , which in turn uses [BlockJUnit4ClassRunner#computeTestMethods](https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java#L138-L145) which only runs tests annotated with `@Test` annotation, hence `@ParamertizedTest` annotated method is being skipped and are not run. will have to override the factory and runner via [UseParametersRunnerFactory](https://junit.org/junit4/javadoc/4.12/org/junit/runners/Parameterized.UseParametersRunnerFactory.html).
Was thinking if it's something worth adding, considering the ROI, really appreciate your feedback's here.
--
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] ajantha-bhat commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r981935957
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
+ for (String scenario : scenarios) {
Review Comment:
Thanks for adding now. Testcase cover all the scenarios.
Instead of these two for loops, we can use Junit's `ParameterizedTest` with `CsvSource`, it will be very clean compared to this. (as we already depend on Junit5)
Example:
https://github.com/projectnessie/nessie/blob/main/clients/spark-extensions-basetests/src/main/java/org/projectnessie/spark/extensions/AbstractNessieSparkSqlExtensionTest.java#L589-L596
--
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] rdblue commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r997275022
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
+ for (String scenario : scenarios) {
+ for (String partitionColType : partitionColTypes) {
+ withSQLConf(
+ ImmutableMap.of("spark.sql.datetime.java8API.enabled", scenario),
+ () -> {
+ String createIceberg =
+ "CREATE TABLE %s (id INTEGER, name STRING, dept STRING, ts %s) USING iceberg PARTITIONED BY (ts)";
+ sql(createIceberg, tableName, partitionColType);
+ try {
+ spark
+ .createDataFrame(
+ ImmutableList.of(
+ RowFactory.create(
+ 1,
+ "John Doe",
+ "hr",
+ partitionColumn(partitionColType, "2021-01-01")),
+ RowFactory.create(
+ 2,
+ "Jane Doe",
+ "hr",
+ partitionColumn(partitionColType, "2021-01-02")),
+ RowFactory.create(
+ 3,
+ "Matt Doe",
+ "hr",
+ partitionColumn(partitionColType, "2021-01-03")),
+ RowFactory.create(
+ 4,
+ "Will Doe",
+ "facilities",
+ partitionColumn(partitionColType, "2021-01-04"))),
+ new StructType(
+ ("DATE".equals(partitionColType) ? dateStruct : timeStampStruct)))
Review Comment:
Can this load the table type from the table rather than creating a static one? Something like this:
```java
sparkSchema = spark.table(tableName).schema()
```
--
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] singhpk234 commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r997766181
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
Review Comment:
> That's the normal case that we know works, right? I guess it's good to have for completeness?
+1, the normal case works, added the normal case for increasing the coverage and completeness as we didn't had UT's for date / timestamp partitioned tables before.
--
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] rdblue commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r997278832
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
Review Comment:
I don't think we really need to test the case where Java 8 date/time classes are disabled. That's the normal case that we know works, right? I guess it's good to have for completeness?
--
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] singhpk234 commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r997874540
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
+ for (String scenario : scenarios) {
Review Comment:
I read though the JUnit's code since, to deep dive on observed behaviour, we are using Parametrized class via SparkCatalogTestBase which by default uses [BlockJUnit4ClassRunnerWithParametersFactory](https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/runners/Parameterized.java#L270) , which in turn uses [BlockJUnit4ClassRunner#computeTestMethods](https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java#L138-L145) which only runs tests annotated with `@Test` annotation, hence `@ParamertizedTest` annotated method is being skipped and are not run. will have to override the factory and runner via [UseParametersRunnerFactory](https://junit.org/junit4/javadoc/4.12/org/junit/runners/Parameterized.UseParametersRunnerFactory.html).
Was thinking if it's that's something worth adding, considering the ROI, really appreciate your feedback's here.
--
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] aokolnychyi commented on pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#issuecomment-1278453412
I'll take a look over the weekend.
--
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] singhpk234 commented on pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#issuecomment-1314689267
Thanks @aokolnychyi @rdblue @ajantha-bhat !!!
Added a pr for backporting it to 3.2 and 3.1 https://github.com/apache/iceberg/pull/6190
--
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] ajantha-bhat commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r981075677
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +90,57 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDatePartitionedTables() {
Review Comment:
Nice PR.
Can we please also add a timestamp testcase as the code modification touches that area? (maybe the same test case with parameterised inputs)?
--
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] rdblue commented on a diff in pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#discussion_r997277769
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java:
##########
@@ -75,6 +96,79 @@ public void testRewriteLargeManifests() {
"Must have 4 manifests", 4, table.currentSnapshot().allManifests(table.io()).size());
}
+ @Test
+ public void testRewriteLargeManifestsWithDateOrTimestampPartitionedTables() {
+ String[] scenarios = new String[] {"true", "false"};
+ String[] partitionColTypes = new String[] {"DATE", "TIMESTAMP"};
+ for (String scenario : scenarios) {
Review Comment:
I'd be fine using `@ParamterizedTest`, but it would also work to just have two test cases that both enable Java 8 date/time types.
--
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] singhpk234 commented on pull request #5860: Spark: Fix QueryFailure when running RewriteManifestProcedure on Date partitioned table
Posted by GitBox <gi...@apache.org>.
singhpk234 commented on PR #5860:
URL: https://github.com/apache/iceberg/pull/5860#issuecomment-1309542196
> I don't like running 4 tests in a loop inside one method when 2 test methods would do fine. Can you please remove the complexity of parameterizing this test?
ACK, removed the loops and added 2 test methods for date partitioned table with datetime java8 enabled and timestamp partitioned table with datetime java8 enabled.
--
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