Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-03 Thread via GitHub 


hudi-bot commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5522517028

   
   ## CI report:
   
   * fc6f23d2a1cef3a2797bc5f4e0715626bdd7cfcd Azure: 
[SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16794)
 
   
   
   Bot commands
 @hudi-bot supports the following commands:
   
- `@hudi-bot run azure` re-run the last Azure build
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-02 Thread via GitHub 


hudi-agent commented on code in PR #19815:
URL: https://github.com/apache/hudi/pull/19815#discussion_r3921656686


##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##


Review Comment:
   πŸ€– **Line 657:** Now that the config is `@Deprecated`/no-op, would it be 
worth also marking the fluent `Builder.parquetOutputTimestampType(...)` setter 
`@Deprecated`? A user calling it via the builder gets a silent no-op with no 
deprecation signal (they never see the config's doc string), which is the same 
confusion this PR is trying to remove.
   
   ⚠️ AI-generated; verify before applying. React πŸ‘/πŸ‘Ž to flag 
quality.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-02 Thread via GitHub 


hudi-bot commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5521405588

   
   ## CI report:
   
   * 81656740e9c0766529bd60044a9fcf881315f5c5 Azure: 
[SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16759)
 
   * fc6f23d2a1cef3a2797bc5f4e0715626bdd7cfcd Azure: 
[PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16794)
 
   
   
   Bot commands
 @hudi-bot supports the following commands:
   
- `@hudi-bot run azure` re-run the last Azure build
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-02 Thread via GitHub 


hudi-bot commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5521349661

   
   ## CI report:
   
   * 81656740e9c0766529bd60044a9fcf881315f5c5 Azure: 
[SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16759)
 
   * fc6f23d2a1cef3a2797bc5f4e0715626bdd7cfcd UNKNOWN
   
   
   Bot commands
 @hudi-bot supports the following commands:
   
- `@hudi-bot run azure` re-run the last Azure build
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-02 Thread via GitHub 


rangareddy commented on code in PR #19815:
URL: https://github.com/apache/hudi/pull/19815#discussion_r3921560612


##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##
@@ -240,7 +240,9 @@ public class HoodieStorageConfig extends HoodieConfig {
   .key("hoodie.parquet.outputtimestamptype")
   .defaultValue("TIMESTAMP_MICROS")
   .markAdvanced()
-  .withDocumentation("Sets spark.sql.parquet.outputTimestampType. Parquet 
timestamp type to use when Spark writes data to Parquet files.");
+  .withDocumentation("Sets spark.sql.parquet.outputTimestampType for the 
Spark row-writer Parquet path (for example bulk insert). "
+  + "Writes that go through the Avro path, such as insert and upsert, 
derive the Parquet timestamp type from the table's "
+  + "Avro schema and are unaffected by this config.");

Review Comment:
   Fixed, and thanks for the probe: I confirmed it independently before 
changing anything. `convertField` (L798-816) takes the unit from 
`resolvedSchema`s `TimePrecision` and `spark.sql.parquet.outputTimestampType` 
appears in that class only at L154 where it is set, never read. The description 
now says no effect since 1.1.0 and points at the writer schema, and the config 
carries `@Deprecated` plus `deprecatedAfter("1.1.0")`.



##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##
@@ -240,7 +240,9 @@ public class HoodieStorageConfig extends HoodieConfig {
   .key("hoodie.parquet.outputtimestamptype")
   .defaultValue("TIMESTAMP_MICROS")
   .markAdvanced()
-  .withDocumentation("Sets spark.sql.parquet.outputTimestampType. Parquet 
timestamp type to use when Spark writes data to Parquet files.");
+  .withDocumentation("Sets spark.sql.parquet.outputTimestampType for the 
Spark row-writer Parquet path (for example bulk insert). "

Review Comment:
   Done, both dropped in this PR rather than a follow-up: the dead 
`hadoopConf.set` at L154 and `HoodieWriteConfig.parquetOutputTimestampType()`. 
The builder setter `HoodieStorageConfig.Builder.parquetOutputTimestampType` 
stays, since it is still how a caller sets the (now deprecated) key.



##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##
@@ -240,7 +240,9 @@ public class HoodieStorageConfig extends HoodieConfig {
   .key("hoodie.parquet.outputtimestamptype")
   .defaultValue("TIMESTAMP_MICROS")
   .markAdvanced()
-  .withDocumentation("Sets spark.sql.parquet.outputTimestampType. Parquet 
timestamp type to use when Spark writes data to Parquet files.");
+  .withDocumentation("Sets spark.sql.parquet.outputTimestampType for the 
Spark row-writer Parquet path (for example bulk insert). "
+  + "Writes that go through the Avro path, such as insert and upsert, 
derive the Parquet timestamp type from the table's "
+  + "Avro schema and are unaffected by this config.");

Review Comment:
   Added, but in `TestHoodieInternalRowParquetWriter` rather than 
`TestHoodieRowParquetWriteSupport`: the latter cannot construct the write 
support because it needs a Spark adapter that is not on the `hudi-spark-client` 
test classpath, as its own class javadoc records. Both directions are pinned, 
`TIMESTAMP_MILLIS` config against a timestamp-micros schema gives MICROS and 
the default against a timestamp-millis schema gives MILLIS.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-02 Thread via GitHub 


voonhous commented on code in PR #19815:
URL: https://github.com/apache/hudi/pull/19815#discussion_r3920941197


##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##
@@ -240,7 +240,9 @@ public class HoodieStorageConfig extends HoodieConfig {
   .key("hoodie.parquet.outputtimestamptype")
   .defaultValue("TIMESTAMP_MICROS")
   .markAdvanced()
-  .withDocumentation("Sets spark.sql.parquet.outputTimestampType. Parquet 
timestamp type to use when Spark writes data to Parquet files.");
+  .withDocumentation("Sets spark.sql.parquet.outputTimestampType for the 
Spark row-writer Parquet path (for example bulk insert). "
+  + "Writes that go through the Avro path, such as insert and upsert, 
derive the Parquet timestamp type from the table's "
+  + "Avro schema and are unaffected by this config.");

Review Comment:
   **blocker:** On master the row-writer path does not honor this config 
either, so the narrowed description is still incorrect. Since #13882 
(4d95b2c2d165, in 1.1.0) `HoodieRowParquetWriteSupport` builds the Parquet 
schema itself and takes MICROS/MILLIS from the writer schema's logical type 
(`convertField`, L800-816); the value copied into the hadoop conf at L154 is 
never read. Verified with a direct write against current snapshot jars: 
`TIMESTAMP_MILLIS` with a `timestamp-micros` schema gives `TIMESTAMP(MICROS)`, 
and the default with a `timestamp-millis` schema gives `TIMESTAMP(MILLIS)`. 
Could we document it as a no-op since 1.1.0 and mark it 
`.deprecatedAfter("1.1.0")`, pointing at the writer schema instead?
   
   ```suggestion
 .withDocumentation("No effect since 1.1.0. Both the Spark row writer 
and the Avro Parquet writer derive the "
 + "Parquet timestamp unit from the writer schema's logical type 
(timestamp-micros or timestamp-millis); "
 + "declare the precision in the writer schema (for example via 
hoodie.write.schema) instead.")
 .deprecatedAfter("1.1.0");
   ```
   
   
   Probe results and why bulk insert used to work
   
   | config | writer schema | parquet footer |
   |---|---|---|
   | `TIMESTAMP_MILLIS` | timestamp-micros | `INT64 TIMESTAMP(MICROS,true)` |
   | `TIMESTAMP_MICROS` (default) | timestamp-millis | `INT64 
TIMESTAMP(MILLIS,true)` |
   | `INT96` | timestamp-micros | `INT64 TIMESTAMP(MICROS,true)` |
   | unset | timestamp-micros | `INT64 TIMESTAMP(MICROS,true)` |
   
   Each row constructs `HoodieRowParquetWriteSupport` from a `HoodieConfig` 
holding `hoodie.avro.schema` plus the config value, writes one `InternalRow` 
through `ParquetWriter`, and reads the footer back. Before #13882 the class 
extended Spark's `ParquetWriteSupport`, whose `init()` reads 
`spark.sql.parquet.outputTimestampType`; that is why bulk insert honored the 
config through 1.0.x and the HUDI-8592 reporter saw it work there. Since 1.1.0 
it extends parquet's `WriteSupport` directly, so the key is set but never read 
on any path.
   
   



##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##
@@ -240,7 +240,9 @@ public class HoodieStorageConfig extends HoodieConfig {
   .key("hoodie.parquet.outputtimestamptype")
   .defaultValue("TIMESTAMP_MICROS")
   .markAdvanced()
-  .withDocumentation("Sets spark.sql.parquet.outputTimestampType. Parquet 
timestamp type to use when Spark writes data to Parquet files.");
+  .withDocumentation("Sets spark.sql.parquet.outputTimestampType for the 
Spark row-writer Parquet path (for example bulk insert). "

Review Comment:
   **minor:** Not blocking. `HoodieRowParquetWriteSupport.java:154` still does 
`hadoopConf.set("spark.sql.parquet.outputTimestampType", ...)` even though 
nothing has read it since #13882, and that line is what makes this description 
(and the HUDI-8592 triage) look right on a code read. 
`HoodieWriteConfig.parquetOutputTimestampType()` has no callers either, as the 
PR body notes. Would it be worth dropping both in this PR rather than a 
follow-up, so the next reader does not reach the same conclusion?
   



##
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##
@@ -240,7 +240,9 @@ public class HoodieStorageConfig extends HoodieConfig {
   .key("hoodie.parquet.outputtimestamptype")
   .defaultValue("TIMESTAMP_MICROS")
   .markAdvanced()
-  .withDocumentation("Sets spark.sql.parquet.outputTimestampType. Parquet 
timestamp type to use when Spark writes data to Parquet files.");
+  .withDocumentation("Sets spark.sql.parquet.outputTimestampType for the 
Spark row-writer Parquet path (for example bulk insert). "
+  + "Writes that go through the Avro path, such as insert and upsert, 
derive the Parquet timestamp type from the table's "
+  + "Avro schema and are unaffected by this config.");

Review Comment:
   **minor:** Not blocking. Nothing under any `src/test`

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-01 Thread via GitHub 


hudi-bot commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5505495914

   
   ## CI report:
   
   * 81656740e9c0766529bd60044a9fcf881315f5c5 Azure: 
[SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16759)
 
   
   
   Bot commands
 @hudi-bot supports the following commands:
   
- `@hudi-bot run azure` re-run the last Azure build
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-01 Thread via GitHub 


codecov-commenter commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5504517972

   ## 
[Codecov](https://app.codecov.io/gh/apache/hudi/pull/19815?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 78.31%. Comparing base 
([`a2788ee`](https://app.codecov.io/gh/apache/hudi/commit/a2788eef922e2375432ead3fe50af7fe5f55faf7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`8165674`](https://app.codecov.io/gh/apache/hudi/commit/81656740e9c0766529bd60044a9fcf881315f5c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   Additional details and impacted files
   
   
   
   ```diff
   @@ Coverage Diff  @@
   ## master   #19815  +/-   ##
   
   - Coverage 78.31%   78.31%   -0.01% 
 Complexity3388733887  
   
 Files  2541 2541  
 Lines141728   141728  
 Branches  1718217182  
   
   - Hits 110993   110991   -2 
   - Misses2303123032   +1 
   - Partials   7704 7705   +1 
   ```
   
   | 
[Components](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Ξ” | |
   |---|---|---|
   | 
[hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `83.65% <100.00%> (ΓΈ)` | |
   | 
[hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `83.20% <ΓΈ> (-0.01%)` | :arrow_down: |
   | 
[hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `85.65% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `73.21% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `74.57% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `15.06% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `70.71% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `75.56% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `79.88% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `83.44% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `65.81% <ΓΈ> (ΓΈ)` | |
   | 
[hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19815/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | `53.96% <ΓΈ> (ΓΈ)` | |
   
   | 
[Flag](https://app.codecov.io/gh/apache/hudi/pull/19815/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Ξ” | |
   |---|---|---|
   |

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-01 Thread via GitHub 


hudi-bot commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5504513059

   
   ## CI report:
   
   * 81656740e9c0766529bd60044a9fcf881315f5c5 Azure: 
[PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16759)
 
   
   
   Bot commands
 @hudi-bot supports the following commands:
   
- `@hudi-bot run azure` re-run the last Azure build
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] docs(config): scope hoodie.parquet.outputtimestamptype to the row-writer path [hudi]

2026-09-01 Thread via GitHub 


hudi-bot commented on PR #19815:
URL: https://github.com/apache/hudi/pull/19815#issuecomment-5504467944

   
   ## CI report:
   
   * 81656740e9c0766529bd60044a9fcf881315f5c5 UNKNOWN
   
   
   Bot commands
 @hudi-bot supports the following commands:
   
- `@hudi-bot run azure` re-run the last Azure build
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]