dongjoon-hyun commented on a change in pull request #882:
URL: https://github.com/apache/orc/pull/882#discussion_r695926553



##########
File path: java/core/src/java/org/apache/orc/impl/WriterImpl.java
##########
@@ -163,64 +162,69 @@ public WriterImpl(FileSystem fs,
         opts.getKeyOverrides());
     needKeyFlush = encryption.length > 0;
 
-    // Set up the physical writer
-    this.physicalWriter = opts.getPhysicalWriter() == null ?
-                              new PhysicalFsWriter(fs, path, opts, encryption) 
:
-                              opts.getPhysicalWriter();
-    unencryptedOptions = physicalWriter.getStreamOptions();
-    OutStream.assertBufferSizeValid(unencryptedOptions.getBufferSize());
-
-    this.callback = opts.getCallback();
-    this.writerVersion = opts.getWriterVersion();
-    bloomFilterVersion = opts.getBloomFilterVersion();
     this.directEncodingColumns = OrcUtils.includeColumns(
         opts.getDirectEncodingColumns(), opts.getSchema());
     dictionaryKeySizeThreshold =
         OrcConf.DICTIONARY_KEY_SIZE_THRESHOLD.getDouble(conf);
+
+    this.callback = opts.getCallback();
     if (callback != null) {
       callbackContext = () -> WriterImpl.this;
     } else {
       callbackContext = null;
     }
+
+    this.useProlepticGregorian = opts.getProlepticGregorian();
     this.writeTimeZone = hasTimestamp(schema);
     this.useUTCTimeZone = opts.getUseUTCTimestamp();
-    this.stripeSize = opts.getStripeSize();
-    this.version = opts.getVersion();
+
     this.encodingStrategy = opts.getEncodingStrategy();
     this.compressionStrategy = opts.getCompressionStrategy();
+
     this.rowIndexStride = opts.getRowIndexStride();
-    this.memoryManager = opts.getMemoryManager();
     buildIndex = rowIndexStride > 0;
+    if (buildIndex && rowIndexStride < MIN_ROW_INDEX_STRIDE) {
+      throw new IllegalArgumentException("Row stride must be at least " +
+          MIN_ROW_INDEX_STRIDE);
+    }
+
+    this.writerVersion = opts.getWriterVersion();
+    this.version = opts.getVersion();
     if (version == OrcFile.Version.FUTURE) {
       throw new IllegalArgumentException("Can not write in a unknown 
version.");
     } else if (version == OrcFile.Version.UNSTABLE_PRE_2_0) {
       LOG.warn("ORC files written in " + version.getName() + " will not be" +
           " readable by other versions of the software. It is only for" +
           " developer testing.");
     }
-    boolean buildBloomFilter = buildIndex;
-    if (version == OrcFile.Version.V_0_11) {
-      /* do not write bloom filters for ORC v11 */
-      buildBloomFilter = false;
-    }
-    if (!buildBloomFilter) {
+
+    this.bloomFilterVersion = opts.getBloomFilterVersion();
+    this.bloomFilterFpp = opts.getBloomFilterFpp();
+    /* do not write bloom filters for ORC v11 */
+    if (!buildIndex || version == OrcFile.Version.V_0_11) {

Review comment:
       Ya, I agree with you that removing `buildBloomFilter` looks better.




-- 
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]


Reply via email to