Enabling Incremental Writes in the core API about parquet4s HOT 6 CLOSED

Hi Alex!

First I would like to thank you for interest in this library and for your wish to contribute!

I think that what you want to add is valuable.
Unfortunately, when designing the writer I didn't think of state. I would even tell I wanted to avoid it. You are right, introducing it would break current API - and we cannot do it just like that, because the library is already in use by many people. I would like to do the release with breaking changes in 1.0.0 version when all features are already there and library ends his child age.

The only idea I have at this moment is just to add a separate trait for a new solution. We can use also Java's Closeable trait, eg.:

object IncrementalParquetWriter {

  def apply[T: ParquetRecordEncoder : ParquetSchemaResolver](path: String, options: ParquetWriter.Options): IncrementalParquetWriter[T] =
    new IncrementalParquetWriter[T] {
      private val writer = internalWriter(new Path(path), ParquetSchemaResolver.resolveSchema[T], options)
      private val valueCodecConfiguration = options.toValueCodecConfiguration

      override def write(data: Iterable[T]): Unit =
        data.foreach { elem =>
          writer.write(ParquetRecordEncoder.encode[T](elem, valueCodecConfiguration))
        }
      override def close(): Unit = writer.close()
    }

}

trait IncrementalParquetWriter[T] extends Closeable {

  def write(data: Iterable[T])

}

I would not modify existing IT tests but add a simple separate one that just checks if new incremental writer works as intended (including edge cases).

from parquet4s.

I see. Thanks, I'll take another pass at that here in a minute. What edge cases do you have in mind here?

from parquet4s.

Cool!
Regarding edge cases I mean:

• Writing with closed reader
• Closing writer without writing anything with it.
• Whatever else you come up with :)

from parquet4s.

Sounds good. I opened a new PR with this implementation. It's similar to my second PR but doesn't mess with all the type parameters.

from parquet4s.

I am merging your PR but let me do some changes to it before release. I would like to replace isClosed var to something more thread-safe. [EDIT] Just noticed that you used synchronised :)

from parquet4s.

Great! Looking forward to the release. Closing this issue. thanks for the quick review and merge.

from parquet4s.