You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "CuteChuanChuan (via GitHub)" <gi...@apache.org> on 2025/06/02 13:24:04 UTC
[PR] Build: Fix errorprone warnings [iceberg]
CuteChuanChuan opened a new pull request, #13217:
URL: https://github.com/apache/iceberg/pull/13217
- UnnecessaryParentheses at ADLSFileIO#136: remove unnecessary parentheses
- ObjectsHashCodePrimitive at DynamicRecordInternalSerializer#234: use Boolean.hashCode instead of Objects.hashCode
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166886030
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicRecordInternalSerializer.java:
##########
@@ -20,7 +20,6 @@
import java.io.IOException;
import java.util.Collections;
-import java.util.Objects;
Review Comment:
does this also need to be removed in the other Flink versions?
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166817583
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreClientImpl.java:
##########
@@ -486,21 +489,28 @@ public List<Tables> list(DatasetReference datasetReference, boolean listAllTable
// tables for us to filter out those results since invoking `getTable` on them would
// correctly raise a `NoSuchIcebergTableException` for being inoperable by this plugin.
if (!listAllTables) {
- tablesStream =
- tablesStream
- .parallel()
- .filter(
- table -> {
- try {
- load(table.getTableReference());
- } catch (NoSuchTableException e) {
- return false;
- }
- return true;
- });
+ List<Tables> allTables = tablesStream.collect(Collectors.toList());
+ List<Tables> validTables = Collections.synchronizedList(Lists.newArrayList());
+
+ Tasks.foreach(allTables)
+ .executeWith(ThreadPools.getWorkerPool())
+ .noRetry()
+ .suppressFailureWhenFinished()
+ .run(
+ table -> {
+ try {
+ load(table.getTableReference());
+ validTables.add(table);
+ } catch (NoSuchTableException e) {
+ // Silently ignore tables that are not valid Iceberg tables
+ // This is expected behavior as we're filtering out non-Iceberg tables
+ }
+ });
+
+ return validTables;
+ } else {
+ return tablesStream.collect(Collectors.toList());
Review Comment:
Thanks for pointing this out!
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166303298
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicRecordInternalSerializer.java:
##########
@@ -231,7 +230,7 @@ public boolean equals(Object obj) {
@Override
public int hashCode() {
- return Objects.hashCode(writeSchemaAndSpec);
+ return Boolean.hashCode(writeSchemaAndSpec);
Review Comment:
can you apply the same across all Flink versions?
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#issuecomment-3018638840
Closes https://github.com/apache/iceberg/issues/13178
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166755769
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicRecordInternalSerializer.java:
##########
@@ -231,7 +230,7 @@ public boolean equals(Object obj) {
@Override
public int hashCode() {
- return Objects.hashCode(writeSchemaAndSpec);
+ return Boolean.hashCode(writeSchemaAndSpec);
Review Comment:
No problem.
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#issuecomment-2976645662
Hi @ajantha-bhat ,
thanks for the heads-up. The nit is solved. Thanks!
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166341883
##########
api/src/main/java/org/apache/iceberg/transforms/Timestamps.java:
##########
@@ -32,24 +32,79 @@
import org.apache.iceberg.util.SerializableFunction;
enum Timestamps implements Transform<Long, Integer> {
- MICROS_TO_YEAR(ChronoUnit.YEARS, "year", MicrosToYears.INSTANCE),
- MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", MicrosToMonths.INSTANCE),
- MICROS_TO_DAY(ChronoUnit.DAYS, "day", MicrosToDays.INSTANCE),
- MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", MicrosToHours.INSTANCE),
+ MICROS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.MICROS),
+ MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.MICROS),
+ MICROS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.MICROS),
+ MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.MICROS),
- NANOS_TO_YEAR(ChronoUnit.YEARS, "year", NanosToYears.INSTANCE),
- NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", NanosToMonths.INSTANCE),
- NANOS_TO_DAY(ChronoUnit.DAYS, "day", NanosToDays.INSTANCE),
- NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", NanosToHours.INSTANCE);
+ NANOS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.NANOS),
+ NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.NANOS),
+ NANOS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.NANOS),
+ NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.NANOS);
+
+ @Immutable
+ static class Apply implements SerializableFunction<Long, Integer> {
+ private final ChronoUnit granularity;
+ private final TimestampUnit timestampUnit;
+
+ Apply(ChronoUnit granularity, TimestampUnit timestampUnit) {
+ this.granularity = granularity;
+ this.timestampUnit = timestampUnit;
+ }
+
+ @Override
+ public Integer apply(Long timestamp) {
+ if (timestamp == null) {
+ return null;
+ }
+
+ switch (timestampUnit) {
+ case MICROS:
+ switch (granularity) {
+ case YEARS:
+ return DateTimeUtil.microsToYears(timestamp);
+ case MONTHS:
+ return DateTimeUtil.microsToMonths(timestamp);
+ case DAYS:
+ return DateTimeUtil.microsToDays(timestamp);
+ case HOURS:
+ return DateTimeUtil.microsToHours(timestamp);
+ default:
+ throw new UnsupportedOperationException("Unsupported time unit: " + granularity);
+ }
+
+ case NANOS:
+ switch (granularity) {
+ case YEARS:
+ return DateTimeUtil.nanosToYears(timestamp);
+ case MONTHS:
+ return DateTimeUtil.nanosToMonths(timestamp);
+ case DAYS:
+ return DateTimeUtil.nanosToDays(timestamp);
+ case HOURS:
+ return DateTimeUtil.nanosToHours(timestamp);
+ default:
+ throw new UnsupportedOperationException("Unsupported time unit: " + granularity);
+ }
+ default:
+ throw new UnsupportedOperationException("Unsupported time unit: " + timestampUnit);
+ }
+ }
+ }
+
+ enum TimestampUnit {
Review Comment:
can you please move this right above `Apply`?
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166830020
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreClientImpl.java:
##########
@@ -485,22 +488,32 @@ public List<Tables> list(DatasetReference datasetReference, boolean listAllTable
// distinguish Iceberg
// tables for us to filter out those results since invoking `getTable` on them would
// correctly raise a `NoSuchIcebergTableException` for being inoperable by this plugin.
+
+ List<Tables> allTables = tablesStream.collect(Collectors.toList());
+
if (!listAllTables) {
- tablesStream =
- tablesStream
- .parallel()
- .filter(
- table -> {
- try {
- load(table.getTableReference());
- } catch (NoSuchTableException e) {
- return false;
- }
- return true;
- });
- }
- return tablesStream.collect(Collectors.toList());
+ List<Tables> validTables = Collections.synchronizedList(Lists.newArrayList());
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#issuecomment-3005008745
Hi @nastra,
Thanks for the feedback. PTAL when you have a chance.
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166781068
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreClientImpl.java:
##########
@@ -486,21 +489,28 @@ public List<Tables> list(DatasetReference datasetReference, boolean listAllTable
// tables for us to filter out those results since invoking `getTable` on them would
// correctly raise a `NoSuchIcebergTableException` for being inoperable by this plugin.
if (!listAllTables) {
- tablesStream =
- tablesStream
- .parallel()
- .filter(
- table -> {
- try {
- load(table.getTableReference());
- } catch (NoSuchTableException e) {
- return false;
- }
- return true;
- });
+ List<Tables> allTables = tablesStream.collect(Collectors.toList());
+ List<Tables> validTables = Collections.synchronizedList(Lists.newArrayList());
+
+ Tasks.foreach(allTables)
+ .executeWith(ThreadPools.getWorkerPool())
+ .noRetry()
+ .suppressFailureWhenFinished()
+ .run(
+ table -> {
+ try {
+ load(table.getTableReference());
+ validTables.add(table);
+ } catch (NoSuchTableException e) {
+ // Silently ignore tables that are not valid Iceberg tables
+ // This is expected behavior as we're filtering out non-Iceberg tables
+ }
+ });
+
+ return validTables;
+ } else {
+ return tablesStream.collect(Collectors.toList());
Review Comment:
Thanks for pointing this out!
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2149552805
##########
api/src/main/java/org/apache/iceberg/transforms/Timestamps.java:
##########
@@ -32,24 +32,78 @@
import org.apache.iceberg.util.SerializableFunction;
enum Timestamps implements Transform<Long, Integer> {
- MICROS_TO_YEAR(ChronoUnit.YEARS, "year", MicrosToYears.INSTANCE),
- MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", MicrosToMonths.INSTANCE),
- MICROS_TO_DAY(ChronoUnit.DAYS, "day", MicrosToDays.INSTANCE),
- MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", MicrosToHours.INSTANCE),
+ MICROS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.MICROS),
+ MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.MICROS),
+ MICROS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.MICROS),
+ MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.MICROS),
- NANOS_TO_YEAR(ChronoUnit.YEARS, "year", NanosToYears.INSTANCE),
- NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", NanosToMonths.INSTANCE),
- NANOS_TO_DAY(ChronoUnit.DAYS, "day", NanosToDays.INSTANCE),
- NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", NanosToHours.INSTANCE);
+ NANOS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.NANOS),
+ NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.NANOS),
+ NANOS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.NANOS),
+ NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.NANOS);
+
+ @Immutable
+ static class Apply implements SerializableFunction<Long, Integer> {
Review Comment:
comment for other reviewers:
To fix errorprone warning, followed the `Dates` enum style.
##########
api/src/main/java/org/apache/iceberg/transforms/Timestamps.java:
##########
@@ -32,24 +32,78 @@
import org.apache.iceberg.util.SerializableFunction;
enum Timestamps implements Transform<Long, Integer> {
- MICROS_TO_YEAR(ChronoUnit.YEARS, "year", MicrosToYears.INSTANCE),
- MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", MicrosToMonths.INSTANCE),
- MICROS_TO_DAY(ChronoUnit.DAYS, "day", MicrosToDays.INSTANCE),
- MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", MicrosToHours.INSTANCE),
+ MICROS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.MICROS),
+ MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.MICROS),
+ MICROS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.MICROS),
+ MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.MICROS),
- NANOS_TO_YEAR(ChronoUnit.YEARS, "year", NanosToYears.INSTANCE),
- NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", NanosToMonths.INSTANCE),
- NANOS_TO_DAY(ChronoUnit.DAYS, "day", NanosToDays.INSTANCE),
- NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", NanosToHours.INSTANCE);
+ NANOS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.NANOS),
+ NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.NANOS),
+ NANOS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.NANOS),
+ NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.NANOS);
+
+ @Immutable
+ static class Apply implements SerializableFunction<Long, Integer> {
+ private final ChronoUnit granularity;
+ private final TimestampUnit timestampUnit;
+
+ Apply(ChronoUnit granularity, TimestampUnit timestampUnit) {
+ this.granularity = granularity;
+ this.timestampUnit = timestampUnit;
+ }
+
+ @Override
+ public Integer apply(Long timestamp) {
+ if (timestamp == null) {
+ return null;
+ }
+
+ switch (timestampUnit) {
+ case MICROS:
+ switch (granularity) {
+ case YEARS:
+ return DateTimeUtil.microsToYears(timestamp);
+ case MONTHS:
+ return DateTimeUtil.microsToMonths(timestamp);
+ case DAYS:
+ return DateTimeUtil.microsToDays(timestamp);
+ case HOURS:
+ return DateTimeUtil.microsToHours(timestamp);
+ default:
+ throw new UnsupportedOperationException("Unsupported time unit: " + granularity);
+ }
Review Comment:
nit: add new lines to separate the block
https://iceberg.apache.org/contribute/?h=block+spacing#block-spacing
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166344149
##########
api/src/main/java/org/apache/iceberg/transforms/Timestamps.java:
##########
@@ -32,24 +32,79 @@
import org.apache.iceberg.util.SerializableFunction;
enum Timestamps implements Transform<Long, Integer> {
- MICROS_TO_YEAR(ChronoUnit.YEARS, "year", MicrosToYears.INSTANCE),
- MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", MicrosToMonths.INSTANCE),
- MICROS_TO_DAY(ChronoUnit.DAYS, "day", MicrosToDays.INSTANCE),
- MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", MicrosToHours.INSTANCE),
+ MICROS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.MICROS),
+ MICROS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.MICROS),
+ MICROS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.MICROS),
+ MICROS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.MICROS),
- NANOS_TO_YEAR(ChronoUnit.YEARS, "year", NanosToYears.INSTANCE),
- NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", NanosToMonths.INSTANCE),
- NANOS_TO_DAY(ChronoUnit.DAYS, "day", NanosToDays.INSTANCE),
- NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", NanosToHours.INSTANCE);
+ NANOS_TO_YEAR(ChronoUnit.YEARS, "year", TimestampUnit.NANOS),
+ NANOS_TO_MONTH(ChronoUnit.MONTHS, "month", TimestampUnit.NANOS),
+ NANOS_TO_DAY(ChronoUnit.DAYS, "day", TimestampUnit.NANOS),
+ NANOS_TO_HOUR(ChronoUnit.HOURS, "hour", TimestampUnit.NANOS);
+
+ @Immutable
+ static class Apply implements SerializableFunction<Long, Integer> {
+ private final ChronoUnit granularity;
+ private final TimestampUnit timestampUnit;
+
+ Apply(ChronoUnit granularity, TimestampUnit timestampUnit) {
+ this.granularity = granularity;
+ this.timestampUnit = timestampUnit;
+ }
+
+ @Override
+ public Integer apply(Long timestamp) {
+ if (timestamp == null) {
+ return null;
+ }
+
+ switch (timestampUnit) {
+ case MICROS:
+ switch (granularity) {
+ case YEARS:
+ return DateTimeUtil.microsToYears(timestamp);
+ case MONTHS:
+ return DateTimeUtil.microsToMonths(timestamp);
+ case DAYS:
+ return DateTimeUtil.microsToDays(timestamp);
+ case HOURS:
+ return DateTimeUtil.microsToHours(timestamp);
+ default:
+ throw new UnsupportedOperationException("Unsupported time unit: " + granularity);
+ }
+
Review Comment:
```suggestion
```
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166350637
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreClientImpl.java:
##########
@@ -486,21 +489,28 @@ public List<Tables> list(DatasetReference datasetReference, boolean listAllTable
// tables for us to filter out those results since invoking `getTable` on them would
// correctly raise a `NoSuchIcebergTableException` for being inoperable by this plugin.
if (!listAllTables) {
- tablesStream =
- tablesStream
- .parallel()
- .filter(
- table -> {
- try {
- load(table.getTableReference());
- } catch (NoSuchTableException e) {
- return false;
- }
- return true;
- });
+ List<Tables> allTables = tablesStream.collect(Collectors.toList());
+ List<Tables> validTables = Collections.synchronizedList(Lists.newArrayList());
+
+ Tasks.foreach(allTables)
+ .executeWith(ThreadPools.getWorkerPool())
+ .noRetry()
+ .suppressFailureWhenFinished()
+ .run(
+ table -> {
+ try {
+ load(table.getTableReference());
+ validTables.add(table);
+ } catch (NoSuchTableException e) {
+ // Silently ignore tables that are not valid Iceberg tables
+ // This is expected behavior as we're filtering out non-Iceberg tables
+ }
+ });
+
+ return validTables;
+ } else {
+ return tablesStream.collect(Collectors.toList());
Review Comment:
```suggestion
return allTables;
```
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#issuecomment-3003704285
@nastra, @pvary: anymore comments for this? +1 from my side.
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#issuecomment-2973565388
Hi @ajantha-bhat,
I've resolved the conflicts and rebased against the latest upstream-main.
There is no error prone warning in build. PTAL when you have a chance.
Thanks!
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2167002503
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicRecordInternalSerializer.java:
##########
@@ -20,7 +20,6 @@
import java.io.IOException;
import java.util.Collections;
-import java.util.Objects;
Review Comment:
You're right. Thanks for this heads-up
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166829121
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreClientImpl.java:
##########
@@ -485,22 +488,32 @@ public List<Tables> list(DatasetReference datasetReference, boolean listAllTable
// distinguish Iceberg
// tables for us to filter out those results since invoking `getTable` on them would
// correctly raise a `NoSuchIcebergTableException` for being inoperable by this plugin.
+
+ List<Tables> allTables = tablesStream.collect(Collectors.toList());
+
if (!listAllTables) {
- tablesStream =
- tablesStream
- .parallel()
- .filter(
- table -> {
- try {
- load(table.getTableReference());
- } catch (NoSuchTableException e) {
- return false;
- }
- return true;
- });
- }
Review Comment:
```suggestion
```
--
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
Re: [PR] Build: Fix errorprone warnings [iceberg]
Posted by "CuteChuanChuan (via GitHub)" <gi...@apache.org>.
CuteChuanChuan commented on code in PR #13217:
URL: https://github.com/apache/iceberg/pull/13217#discussion_r2166781068
##########
bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreClientImpl.java:
##########
@@ -486,21 +489,28 @@ public List<Tables> list(DatasetReference datasetReference, boolean listAllTable
// tables for us to filter out those results since invoking `getTable` on them would
// correctly raise a `NoSuchIcebergTableException` for being inoperable by this plugin.
if (!listAllTables) {
- tablesStream =
- tablesStream
- .parallel()
- .filter(
- table -> {
- try {
- load(table.getTableReference());
- } catch (NoSuchTableException e) {
- return false;
- }
- return true;
- });
+ List<Tables> allTables = tablesStream.collect(Collectors.toList());
+ List<Tables> validTables = Collections.synchronizedList(Lists.newArrayList());
+
+ Tasks.foreach(allTables)
+ .executeWith(ThreadPools.getWorkerPool())
+ .noRetry()
+ .suppressFailureWhenFinished()
+ .run(
+ table -> {
+ try {
+ load(table.getTableReference());
+ validTables.add(table);
+ } catch (NoSuchTableException e) {
+ // Silently ignore tables that are not valid Iceberg tables
+ // This is expected behavior as we're filtering out non-Iceberg tables
+ }
+ });
+
+ return validTables;
+ } else {
+ return tablesStream.collect(Collectors.toList());
Review Comment:
Thanks for pointing this out!
--
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