[GitHub] [incubator-iceberg] aokolnychyi commented on issue #467: Change method signature of ManifestReader::read (#459)

2019-10-11 Thread GitBox
aokolnychyi commented on issue #467: Change method signature of ManifestReader::read (#459) URL: https://github.com/apache/incubator-iceberg/pull/467#issuecomment-540988619 I am +1. I think it is in line with #511 and will eliminate the need to build a spec lookup function in utilities. -

[GitHub] [incubator-iceberg] Fokko commented on issue #526: Add Baseline to iceberg-parquet

2019-10-11 Thread GitBox
Fokko commented on issue #526: Add Baseline to iceberg-parquet URL: https://github.com/apache/incubator-iceberg/pull/526#issuecomment-541108885 Thanks @rdsr for pointing out, I've fixed the checkstyle issues as well. This is a

[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdblue commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334077328 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.

[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #497: Support retaining last N snapshots

2019-10-11 Thread GitBox
rdblue commented on a change in pull request #497: Support retaining last N snapshots URL: https://github.com/apache/incubator-iceberg/pull/497#discussion_r334080849 ## File path: core/src/main/java/org/apache/iceberg/RemoveSnapshots.java ## @@ -77,8 +79,34 @@ public Expir

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334081033 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.ja

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334081033 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.ja

[GitHub] [incubator-iceberg] rdblue commented on issue #497: Support retaining last N snapshots

2019-10-11 Thread GitBox
rdblue commented on issue #497: Support retaining last N snapshots URL: https://github.com/apache/incubator-iceberg/pull/497#issuecomment-541140549 @yathindranath, I think we should add table properties in a separate PR. For this one, let's focus on making sure we have a good plan for the b

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334083246 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.ja

[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdblue commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334087328 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.

[GitHub] [incubator-iceberg] rdblue opened a new pull request #538: Fix connection leak in Hive table tests

2019-10-11 Thread GitBox
rdblue opened a new pull request #538: Fix connection leak in Hive table tests URL: https://github.com/apache/incubator-iceberg/pull/538 This is another attempt to fix the connection leak in the Hive table tests. The current theory is that the cache in `HiveCatalogs` is not behaving correc

[GitHub] [incubator-iceberg] yathindranath commented on a change in pull request #497: Support retaining last N snapshots

2019-10-11 Thread GitBox
yathindranath commented on a change in pull request #497: Support retaining last N snapshots URL: https://github.com/apache/incubator-iceberg/pull/497#discussion_r334128531 ## File path: core/src/main/java/org/apache/iceberg/RemoveSnapshots.java ## @@ -77,8 +79,34 @@ publi

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334136688 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.ja

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334136688 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.ja

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#discussion_r334139857 ## File path: core/src/test/java/org/apache/iceberg/avro/TestAvroNameMapping.ja

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334147493 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -51,13 +54,13 @@ public Hadoop

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334143388 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -20,27 +20,30 @@ package org.

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334144240 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -51,13 +54,13 @@ public Hadoop

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334144615 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -20,27 +20,30 @@ package org.

[GitHub] [incubator-iceberg] rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
rdsr commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334147228 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -20,27 +20,30 @@ package org.

[GitHub] [incubator-iceberg] andrei-ionescu commented on issue #510: Cannot update an Iceberg dataset from a Parquet file due to "field should be required, but is optional"

2019-10-11 Thread GitBox
andrei-ionescu commented on issue #510: Cannot update an Iceberg dataset from a Parquet file due to "field should be required, but is optional" URL: https://github.com/apache/incubator-iceberg/issues/510#issuecomment-541201187 Thank you @rdblue, I’ll add the changes soon on the https://gi

[GitHub] [incubator-iceberg] andrei-ionescu commented on issue #514: Fix for cannot update an Iceberg dataset from a Parquet file (#510)

2019-10-11 Thread GitBox
andrei-ionescu commented on issue #514: Fix for cannot update an Iceberg dataset from a Parquet file (#510) URL: https://github.com/apache/incubator-iceberg/pull/514#issuecomment-541202306 Thank you @rdblue. I’ll do that. Thi

[GitHub] [incubator-iceberg] manishmalhotrawork commented on issue #499: Add persistent IDs to partition fields (WIP)

2019-10-11 Thread GitBox
manishmalhotrawork commented on issue #499: Add persistent IDs to partition fields (WIP) URL: https://github.com/apache/incubator-iceberg/pull/499#issuecomment-541204076 > Yeah, the first step is to add a partition field ID in addition to the existing source field ID. Cool. then we also

[GitHub] [incubator-iceberg] rdblue commented on issue #499: Add persistent IDs to partition fields (WIP)

2019-10-11 Thread GitBox
rdblue commented on issue #499: Add persistent IDs to partition fields (WIP) URL: https://github.com/apache/incubator-iceberg/pull/499#issuecomment-541212272 > then we also have to also handle cases where table is created with old way (partition-field if in ) and not adding partitionSpec to

[GitHub] [incubator-iceberg] rdblue commented on issue #538: Fix connection leak in Hive table tests

2019-10-11 Thread GitBox
rdblue commented on issue #538: Fix connection leak in Hive table tests URL: https://github.com/apache/incubator-iceberg/pull/538#issuecomment-541213614 Tests have passed twice so far. Starting a third run. This is an automate

[GitHub] [incubator-iceberg] manishmalhotrawork commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
manishmalhotrawork commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334164158 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -0,0 +1,142 @@

[GitHub] [incubator-iceberg] manishmalhotrawork commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
manishmalhotrawork commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334163368 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -0,0 +1,142 @@

[GitHub] [incubator-iceberg] manishmalhotrawork commented on a change in pull request #514: Fix for cannot update an Iceberg dataset from a Parquet file (#510)

2019-10-11 Thread GitBox
manishmalhotrawork commented on a change in pull request #514: Fix for cannot update an Iceberg dataset from a Parquet file (#510) URL: https://github.com/apache/incubator-iceberg/pull/514#discussion_r334157931 ## File path: spark/src/test/java/org/apache/iceberg/spark/source/TestN

[GitHub] [incubator-iceberg] ppadma commented on issue #491: Use relative path for manifest_path and file_path

2019-10-11 Thread GitBox
ppadma commented on issue #491: Use relative path for manifest_path and file_path URL: https://github.com/apache/incubator-iceberg/pull/491#issuecomment-541221493 @rdblue We have a use case where we have to stitch data in two different data lakes as a single table. We already have an iceb

[GitHub] [incubator-iceberg] rdblue commented on issue #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdblue commented on issue #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#issuecomment-541222346 > If that is the case, then we are effectively changing the projected/read schema . . . Won't this aff

[GitHub] [incubator-iceberg] rdblue commented on issue #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdblue commented on issue #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#issuecomment-541222642 > if we maintain the invariant that the read schema is always a common subset of the table schema and

[GitHub] [incubator-iceberg] aokolnychyi merged pull request #538: Fix connection leak in Hive table tests

2019-10-11 Thread GitBox
aokolnychyi merged pull request #538: Fix connection leak in Hive table tests URL: https://github.com/apache/incubator-iceberg/pull/538 This is an automated message from the Apache Git Service. To respond to the message, plea

[GitHub] [incubator-iceberg] manishmalhotrawork edited a comment on issue #499: Add persistent IDs to partition fields (WIP)

2019-10-11 Thread GitBox
manishmalhotrawork edited a comment on issue #499: Add persistent IDs to partition fields (WIP) URL: https://github.com/apache/incubator-iceberg/pull/499#issuecomment-541204076 > Yeah, the first step is to add a partition field ID in addition to the existing source field ID. Cool. t

[GitHub] [incubator-iceberg] rdsr commented on issue #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr commented on issue #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#issuecomment-541275065 Thanks for the detailed. Explanation. I'll try this out. ---

[GitHub] [incubator-iceberg] chenjunjiedada commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
chenjunjiedada commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334216827 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -20,27 +20,30 @@ p

[GitHub] [incubator-iceberg] rdsr edited a comment on issue #207: Add external schema mappings for files written with name-based schemas #40

2019-10-11 Thread GitBox
rdsr edited a comment on issue #207: Add external schema mappings for files written with name-based schemas #40 URL: https://github.com/apache/incubator-iceberg/pull/207#issuecomment-541275065 Thanks for the detailed explanation. I'll try this out. -

[GitHub] [incubator-iceberg] chenjunjiedada commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
chenjunjiedada commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334218172 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -0,0 +1,142 @@ +/*

[GitHub] [incubator-iceberg] chenjunjiedada commented on a change in pull request #529: Add hadoop table catalog (WIP)

2019-10-11 Thread GitBox
chenjunjiedada commented on a change in pull request #529: Add hadoop table catalog (WIP) URL: https://github.com/apache/incubator-iceberg/pull/529#discussion_r334218376 ## File path: core/src/main/java/org/apache/iceberg/hadoop/HadoopCatalog.java ## @@ -0,0 +1,142 @@ +/*

[GitHub] [incubator-iceberg] linxingyuan1102 commented on issue #467: Change method signature of ManifestReader::read (#459)

2019-10-11 Thread GitBox
linxingyuan1102 commented on issue #467: Change method signature of ManifestReader::read (#459) URL: https://github.com/apache/incubator-iceberg/pull/467#issuecomment-541290098 @rdblue @aokolnychyi Thank you for the review! I'll continue working on steps 2&3 listed in #459 after this PR is

[GitHub] [incubator-iceberg] Fokko opened a new pull request #539: Replace StringBuffer by StringBuilder

2019-10-11 Thread GitBox
Fokko opened a new pull request #539: Replace StringBuffer by StringBuilder URL: https://github.com/apache/incubator-iceberg/pull/539 Fixes reported warning: ``` incubator-iceberg/api/src/main/java/org/apache/iceberg/util/UnicodeUtil.java:44: warning: [JdkObsolete] StringBuffer pe