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]
