[
https://issues.apache.org/jira/browse/DRILL-1282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17447978#comment-17447978
]
ASF GitHub Bot commented on DRILL-1282:
---------------------------------------
vdiravka commented on a change in pull request #2351:
URL: https://github.com/apache/drill/pull/2351#discussion_r755060172
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetReaderConfig.java
##########
@@ -186,11 +187,13 @@ public ParquetReaderConfig build() {
readerConfig.enableTimeReadCounter =
conf.getBoolean(ENABLE_TIME_READ_COUNTER, readerConfig.enableTimeReadCounter);
}
- // last assign values from session options, session options have higher
priority than other configurations
+ // last assign values from session or query scoped options which have
higher priority than other configurations
if (options != null) {
- String option =
options.getOption(ExecConstants.PARQUET_READER_STRINGS_SIGNED_MIN_MAX_VALIDATOR);
- if (!option.isEmpty()) {
- readerConfig.enableStringsSignedMinMax = Boolean.valueOf(option);
+ String optVal = (String) options.getOption(
+ ExecConstants.PARQUET_READER_STRINGS_SIGNED_MIN_MAX
+ ).getValueMinScope(OptionValue.OptionScope.SESSION);
Review comment:
Sounds reasonable. Possibly this logic will be migrated to
`EffectiveConfigResolver` eventually
##########
File path:
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestParquetWriter.java
##########
@@ -137,14 +140,14 @@ public static void setupTestFiles() {
public int repeat = 1;
@BeforeClass
Review comment:
For sure we can still use JUnit4 here, but interested have you tries
JUnit5?
I suppose just updating annotations is required for that, for instance
`@BeforeClass` -> `@BeforeAll`..
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java
##########
@@ -141,40 +145,94 @@ public AbstractWriter getWriter(PhysicalOperator child,
String location, List<St
}
public RecordWriter getRecordWriter(FragmentContext context, ParquetWriter
writer) throws IOException, OutOfMemoryException {
- Map<String, String> options = new HashMap<>();
+ Map<String, String> writerOpts = new HashMap<>();
+ OptionManager contextOpts = context.getOptions();
- options.put("location", writer.getLocation());
+ writerOpts.put("location", writer.getLocation());
FragmentHandle handle = context.getHandle();
String fragmentId = String.format("%d_%d", handle.getMajorFragmentId(),
handle.getMinorFragmentId());
- options.put("prefix", fragmentId);
-
- options.put(ExecConstants.PARQUET_BLOCK_SIZE,
context.getOptions().getOption(ExecConstants.PARQUET_BLOCK_SIZE).num_val.toString());
- options.put(ExecConstants.PARQUET_WRITER_USE_SINGLE_FS_BLOCK,
-
context.getOptions().getOption(ExecConstants.PARQUET_WRITER_USE_SINGLE_FS_BLOCK).bool_val.toString());
- options.put(ExecConstants.PARQUET_PAGE_SIZE,
context.getOptions().getOption(ExecConstants.PARQUET_PAGE_SIZE).num_val.toString());
- options.put(ExecConstants.PARQUET_DICT_PAGE_SIZE,
context.getOptions().getOption(ExecConstants.PARQUET_DICT_PAGE_SIZE).num_val.toString());
-
- options.put(ExecConstants.PARQUET_WRITER_COMPRESSION_TYPE,
-
context.getOptions().getOption(ExecConstants.PARQUET_WRITER_COMPRESSION_TYPE).string_val);
-
- options.put(ExecConstants.PARQUET_WRITER_ENABLE_DICTIONARY_ENCODING,
-
context.getOptions().getOption(ExecConstants.PARQUET_WRITER_ENABLE_DICTIONARY_ENCODING).bool_val.toString());
-
- options.put(ExecConstants.PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS,
-
context.getOptions().getOption(ExecConstants.PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS).string_val);
-
- options.put(ExecConstants.PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS,
-
context.getOptions().getOption(ExecConstants.PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS).bool_val.toString());
+ writerOpts.put("prefix", fragmentId);
+
+ // Many options which follow may be set as Drill config options or in the
parquet format
+ // plugin config. If there is a Drill option set at session scope or
narrower it takes precendence.
+ OptionValue.OptionScope minScope = OptionValue.OptionScope.SESSION;
Review comment:
Sounds good. In Drill V2 we can define the motivation for using SESSION,
QUERY and FORMAT options. And we can discuss whether we need to manage format
config options via SESSION options.
At least we are fully free with choosing the best behavior for Drill users
in spite of backward compatibility
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java
##########
@@ -78,6 +81,9 @@
public class ParquetFormatPlugin implements FormatPlugin {
+ // {@link org.apache.parquet.column.ParquetProperties#WriterVersion}
Review comment:
It can be added via javadoc so `@link` will allow to use reference as a
hyperlink in IDE
--
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: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Add read and write support for Parquet v2
> -----------------------------------------
>
> Key: DRILL-1282
> URL: https://issues.apache.org/jira/browse/DRILL-1282
> Project: Apache Drill
> Issue Type: Improvement
> Components: Storage - Parquet
> Reporter: Jacques Nadeau
> Assignee: James Turton
> Priority: Minor
> Fix For: Future
>
>
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
