[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/14419 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73395973
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,18 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
+class ParquetWriterBuilder() extends
+ParquetWriter.Builder[RecordConsumer => Unit,
ParquetWriterBuilder](new Path(path)) {
+ @Override def getWriteSupport(conf:
org.apache.hadoop.conf.Configuration) = {
--- End diff --
sure :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73373242
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
Ack yeah that's what I meant, thank you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73367655
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
scala just do override, not even @override.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73331335
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
Filed here, https://issues.apache.org/jira/browse/SPARK-16877 but I will do
some researches and take a look into this deeper before opening a PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73328549
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
Sure, because I don't think it would actually cause the Scala compiler to
verify it overrides. It's not illegal to use a Java annotation, just doesn't do
anything in this case?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73327929
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
I didn't know Java annotation passes Scala style checking. Should we
consider adding a rule for this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73320252
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,18 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
+class ParquetWriterBuilder() extends
+ParquetWriter.Builder[RecordConsumer => Unit,
ParquetWriterBuilder](new Path(path)) {
+ @Override def getWriteSupport(conf:
org.apache.hadoop.conf.Configuration) = {
--- End diff --
Import `Configuration`? and maybe just write the trivial body of both
methods inline without braces on one line. Just personal taste really, to keep
this anonymous class compact
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73320096
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
You might just go ahead and add it as a comment for good measure.
Isn't `@Override` the _Java_ annotation? thought Scala needed `@override`
but could be missing something.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r73106592
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,19 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
--- End diff --
@rxin Does this explanation make sense for you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r72915861
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,18 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
+case class ParquetWriterBuilder() extends
--- End diff --
Thats a good point, they probably don't need to be case classes (just
happened to write that way when I started the code) - I'll switch them to
regular classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r72914240
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,18 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
+case class ParquetWriterBuilder() extends
--- End diff --
Ah, you want to just block that this class is inherited accidentally just
in case?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14419: [SPARK-16814][SQL] Fix deprecated parquet constru...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/14419#discussion_r72914197
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetCompatibilityTest.scala
---
@@ -119,8 +119,18 @@ private[sql] object ParquetCompatibilityTest {
metadata: Map[String, String],
recordWriters: (RecordConsumer => Unit)*): Unit = {
val messageType = MessageTypeParser.parseMessageType(schema)
-val writeSupport = new DirectWriteSupport(messageType, metadata)
-val parquetWriter = new ParquetWriter[RecordConsumer => Unit](new
Path(path), writeSupport)
+val testWriteSupport = new DirectWriteSupport(messageType, metadata)
+case class ParquetWriterBuilder() extends
--- End diff --
One thing I am a bit wondering though is why this has to be `case class`
instead of just `class`. Because it seems it is self-contained, so it seems not
have to be serializable and it seems there is no pattern matching with this.
Just curious.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
