[GitHub] ilooner closed pull request #1508: DRILL-6804: Simplify usage of OperatorPhase in HashAgg.

2018-10-23 Thread GitBox
ilooner closed pull request #1508: DRILL-6804: Simplify usage of OperatorPhase 
in HashAgg.
URL: https://github.com/apache/drill/pull/1508
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
index 80d25edb13a..485d3637277 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java
@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.apache.drill.exec.planner.physical.AggPrelBase;
 import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
 import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.expression.ErrorCollector;
@@ -49,7 +50,6 @@
 import org.apache.drill.exec.physical.impl.common.Comparator;
 import org.apache.drill.exec.physical.impl.common.HashTable;
 import org.apache.drill.exec.physical.impl.common.HashTableConfig;
-import org.apache.drill.exec.planner.physical.AggPrelBase;
 import org.apache.drill.exec.record.AbstractRecordBatch;
 import org.apache.drill.exec.record.BatchSchema;
 import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
@@ -192,9 +192,10 @@ public HashAggBatch(HashAggregate popConfig, RecordBatch 
incoming, FragmentConte
 long memAvail = oContext.getAllocator().getLimit();
 long minBatchesPerPartition = 
context.getOptions().getOption(ExecConstants.HASHAGG_MIN_BATCHES_PER_PARTITION_VALIDATOR);
 long minBatchesNeeded = 2 * minBatchesPerPartition; // 2 - to cover 
overheads, etc.
-boolean is2ndPhase = popConfig.getAggPhase() == 
AggPrelBase.OperatorPhase.PHASE_2of2;
 boolean fallbackEnabled = 
context.getOptions().getOption(ExecConstants.HASHAGG_FALLBACK_ENABLED_KEY).bool_val;
-if ( is2ndPhase && !fallbackEnabled ) {
+final AggPrelBase.OperatorPhase phase = popConfig.getAggPhase();
+
+if ( phase.is2nd() && !fallbackEnabled ) {
   minBatchesNeeded *= 2;  // 2nd phase (w/o fallback) needs at least 2 
partitions
 }
 if ( configuredBatchSize > memAvail / minBatchesNeeded ) { // no cast - 
memAvail may be bigger than max-int
diff --git 
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
 
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
index 6709cf6ddef..32db9eaf487 100644
--- 
a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
+++ 
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
@@ -106,9 +106,7 @@
   private int rowsSpilledReturned = 0;
   private int rowsReturnedEarly = 0;
 
-  private boolean isTwoPhase = false; // 1 phase or 2 phase aggr?
-  private boolean is2ndPhase = false;
-  private boolean is1stPhase = false;
+  private AggPrelBase.OperatorPhase phase;
   private boolean canSpill = true; // make it false in case can not 
spill/return-early
   private ChainedHashTable baseHashTable;
   private boolean earlyOutput = false; // when 1st phase returns a partition 
due to no memory
@@ -379,11 +377,8 @@ public void setup(HashAggregate hashAggrConfig, 
HashTableConfig htConfig, Fragme
 this.outgoing = outgoing;
 this.outContainer = outContainer;
 this.useMemoryPrediction = 
context.getOptions().getOption(ExecConstants.HASHAGG_USE_MEMORY_PREDICTION_VALIDATOR);
-
-is2ndPhase = hashAggrConfig.getAggPhase() == 
AggPrelBase.OperatorPhase.PHASE_2of2;
-isTwoPhase = hashAggrConfig.getAggPhase() != 
AggPrelBase.OperatorPhase.PHASE_1of1;
-is1stPhase = isTwoPhase && !is2ndPhase;
-canSpill = isTwoPhase; // single phase can not spill
+this.phase = hashAggrConfig.getAggPhase();
+canSpill = phase.hasTwo(); // single phase can not spill
 
 // Typically for testing - force a spill after a partition has more than 
so many batches
 minBatchesPerPartition = 
context.getOptions().getOption(ExecConstants.HASHAGG_MIN_BATCHES_PER_PARTITION_VALIDATOR);
@@ -447,7 +442,7 @@ private void delayedSetup() {
 
 // Set the number of partitions from the configuration (raise to a power 
of two, if needed)
 int numPartitions = 
(int)context.getOptions().getOption(ExecConstants.HASHAGG_NUM_PARTITIONS_VALIDATOR);
-if ( numPartitions == 1 && is2ndPhase  ) { // 1st phase can still do early 
return with 1 partition
+if ( numPartitions == 1 && phase.is2nd() ) { // 1st phase can still do 
early return with 1 partition
   canSpill = false;
   logger.warn("Spilling is disabled du

[GitHub] ilooner-mapr commented on a change in pull request #1508: DRILL-6804: Simplify usage of OperatorPhase in HashAgg.

2018-10-23 Thread GitBox
ilooner-mapr commented on a change in pull request #1508: DRILL-6804: Simplify 
usage of OperatorPhase in HashAgg.
URL: https://github.com/apache/drill/pull/1508#discussion_r227583546
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/AggPrelBase.java
 ##
 @@ -47,7 +47,39 @@
 
 public abstract class AggPrelBase extends DrillAggregateRelBase implements 
Prel {
 
-  public enum OperatorPhase {PHASE_1of1, PHASE_1of2, PHASE_2of2}
+  public enum OperatorPhase {
+PHASE_1of1(false, true, "Single"),
+PHASE_1of2(true, true, "1st"),
+PHASE_2of2(true, false, "2nd");
+
+private boolean hasTwo;
+private boolean is1st;
+private String name;
+
+OperatorPhase(boolean hasTwo,
+  boolean is1st,
+  String name) {
+  this.hasTwo = hasTwo;
+  this.is1st = is1st;
+  this.name = name;
+}
+
+public boolean hasTwo() {
+  return hasTwo;
+}
+
+public boolean is1st() {
+  return is1st;
 
 Review comment:
   Thanks for catching this, fixed. And thanks for the review.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


Re: [ANNOUNCE] New Committer: Gautam Parai

2018-10-23 Thread Chunhui Shi
Congrats Gautam!
--
From:Gautam Parai 
Send Time:2018 Oct 22 (Mon) 18:12
To:dev 
Subject:Re: [ANNOUNCE] New Committer: Gautam Parai

Thank you so much all! I hope to continue contributing to the Drill
community even more :)

Gautam

On Mon, Oct 22, 2018 at 5:27 PM weijie tong  wrote:

> Congratulations Gautam !
>
> On Tue, Oct 23, 2018 at 6:28 AM Aman Sinha  wrote:
>
> > Congratulations Gautam !
> >
> > On Mon, Oct 22, 2018 at 3:00 PM Jyothsna Reddy 
> > wrote:
> >
> > > Congrats Gautam!!
> > >
> > >
> > >
> > > On Mon, Oct 22, 2018 at 2:01 PM Vitalii Diravka 
> > > wrote:
> > >
> > > > Congratulations!
> > > >
> > > > On Mon, Oct 22, 2018 at 10:54 PM Khurram Faraaz 
> > > wrote:
> > > >
> > > > > Congrats Gautam!
> > > > >
> > > > > On Mon, Oct 22, 2018 at 10:29 AM Abhishek Girish <
> agir...@apache.org
> > >
> > > > > wrote:
> > > > >
> > > > > > Congrats Gautam!
> > > > > >
> > > > > > On Mon, Oct 22, 2018 at 10:19 AM Karthikeyan Manivannan <
> > > > > > kmanivan...@mapr.com> wrote:
> > > > > >
> > > > > > > Congrats !
> > > > > > >
> > > > > > > On Mon, Oct 22, 2018 at 10:07 AM Kunal Khatua <
> ku...@apache.org>
> > > > > wrote:
> > > > > > >
> > > > > > > > Congratulations, Gautam!
> > > > > > > > On 10/22/2018 10:02:46 AM, Paul Rogers
> >  > > >
> > > > > > > wrote:
> > > > > > > > Congrats Guatam!
> > > > > > > >
> > > > > > > > - Paul
> > > > > > > >
> > > > > > > > Sent from my iPhone
> > > > > > > >
> > > > > > > > > On Oct 22, 2018, at 8:46 AM, salim achouche wrote:
> > > > > > > > >
> > > > > > > > > Congrats Gautam!
> > > > > > > > >
> > > > > > > > >> On Mon, Oct 22, 2018 at 7:25 AM Arina Ielchiieva wrote:
> > > > > > > > >>
> > > > > > > > >> The Project Management Committee (PMC) for Apache Drill
> has
> > > > > invited
> > > > > > > > Gautam
> > > > > > > > >> Parai to become a committer, and we are pleased to
> announce
> > > that
> > > > > he
> > > > > > > has
> > > > > > > > >> accepted.
> > > > > > > > >>
> > > > > > > > >> Gautam has become a contributor since 2016, making changes
> > in
> > > > > > various
> > > > > > > > Drill
> > > > > > > > >> areas including planning side. He is also one of the
> > > > contributors
> > > > > of
> > > > > > > the
> > > > > > > > >> upcoming feature to support index based planning and
> > > execution.
> > > > > > > > >>
> > > > > > > > >> Welcome Gautam, and thank you for your contributions!
> > > > > > > > >>
> > > > > > > > >> - Arina
> > > > > > > > >> (on behalf of Drill PMC)
> > > > > > > > >>
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Regards,
> > > > > > > > > Salim
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>


[GitHub] kkhatua commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
kkhatua commented on a change in pull request #1507: DRILL-6715: Update 
descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227495255
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -145,23 +156,28 @@ private ExecConstants() {
 
   // Hash Aggregate Options
   public static final String HASHAGG_NUM_PARTITIONS_KEY = 
"exec.hashagg.num_partitions";
-  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128, null); // 1 means - no 
spilling
+  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128,
+  new OptionDescription("Sets the initial number of internal partitions 
for Hash Aggregates. Default is 32. May reduce when memory is too small. 
Disables spilling if set to 1.")); // 1 means - no spilling
   public static final String HASHAGG_MAX_MEMORY_KEY = "exec.hashagg.mem_limit";
-  public static final LongValidator HASHAGG_MAX_MEMORY_VALIDATOR = new 
RangeLongValidator(HASHAGG_MAX_MEMORY_KEY, 0, Integer.MAX_VALUE, null);
+  public static final LongValidator HASHAGG_MAX_MEMORY_VALIDATOR = new 
RangeLongValidator(HASHAGG_MAX_MEMORY_KEY, 0, Integer.MAX_VALUE,
+  new OptionDescription("Enforces the value set as the maximum memory for 
the Hash Aggregates. Default is 0 (disabled)."));
 
 Review comment:
   It looks hard to put a description without a  full explanation, which would 
be too much. I'll leave it with some details and suggestion to refer to the 
spill-to-disk feature.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] kkhatua commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
kkhatua commented on a change in pull request #1507: DRILL-6715: Update 
descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227489287
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -145,23 +156,28 @@ private ExecConstants() {
 
   // Hash Aggregate Options
   public static final String HASHAGG_NUM_PARTITIONS_KEY = 
"exec.hashagg.num_partitions";
-  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128, null); // 1 means - no 
spilling
+  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128,
+  new OptionDescription("Sets the initial number of internal partitions 
for Hash Aggregates. Default is 32. May reduce when memory is too small. 
Disables spilling if set to 1.")); // 1 means - no spilling
   public static final String HASHAGG_MAX_MEMORY_KEY = "exec.hashagg.mem_limit";
-  public static final LongValidator HASHAGG_MAX_MEMORY_VALIDATOR = new 
RangeLongValidator(HASHAGG_MAX_MEMORY_KEY, 0, Integer.MAX_VALUE, null);
+  public static final LongValidator HASHAGG_MAX_MEMORY_VALIDATOR = new 
RangeLongValidator(HASHAGG_MAX_MEMORY_KEY, 0, Integer.MAX_VALUE,
+  new OptionDescription("Enforces the value set as the maximum memory for 
the Hash Aggregates. Default is 0 (disabled)."));
 
 Review comment:
   That would be a lot of content in the description. I'll see if I can trim it 
down without losing context.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] kkhatua commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
kkhatua commented on a change in pull request #1507: DRILL-6715: Update 
descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227486402
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -115,25 +116,35 @@ private ExecConstants() {
 
   // Hash Join Options
   public static final String HASHJOIN_HASHTABLE_CALC_TYPE_KEY = 
"exec.hashjoin.hash_table_calc_type";
-  public static final StringValidator HASHJOIN_HASHTABLE_CALC_TYPE = new 
StringValidator(HASHJOIN_HASHTABLE_CALC_TYPE_KEY, null);
+  public static final EnumeratedStringValidator HASHJOIN_HASHTABLE_CALC_TYPE = 
new EnumeratedStringValidator(HASHJOIN_HASHTABLE_CALC_TYPE_KEY,
+  new OptionDescription("Sets the Hash Join Memory Calculator type. 
Default is LEAN. This option also accepts CONSERVATIVE as a value."),
+  "LEAN", "CONSERVATIVE");
   public static final String HASHJOIN_SAFETY_FACTOR_KEY = 
"exec.hashjoin.safety_factor";
-  public static final DoubleValidator HASHJOIN_SAFETY_FACTOR = new 
RangeDoubleValidator(HASHJOIN_SAFETY_FACTOR_KEY, 1.0, Double.MAX_VALUE, null);
+  public static final DoubleValidator HASHJOIN_SAFETY_FACTOR = new 
RangeDoubleValidator(HASHJOIN_SAFETY_FACTOR_KEY, 1.0, Double.MAX_VALUE,
+  new OptionDescription("Sets the Hash Join Memory Calculation Safety; 
multiplies the internal size estimate. Default is 1.0"));
   public static final String HASHJOIN_HASH_DOUBLE_FACTOR_KEY = 
"exec.hashjoin.hash_double_factor";
-  public static final DoubleValidator HASHJOIN_HASH_DOUBLE_FACTOR = new 
RangeDoubleValidator(HASHJOIN_HASH_DOUBLE_FACTOR_KEY, 1.0, Double.MAX_VALUE, 
null);
+  public static final DoubleValidator HASHJOIN_HASH_DOUBLE_FACTOR = new 
RangeDoubleValidator(HASHJOIN_HASH_DOUBLE_FACTOR_KEY, 1.0, Double.MAX_VALUE,
+  new OptionDescription("Sets the Hash Join Memory Calculation; doubling 
factor for the Hash-Table. Default is 2.0"));
   public static final String HASHJOIN_FRAGMENTATION_FACTOR_KEY = 
"exec.hashjoin.fragmentation_factor";
-  public static final DoubleValidator HASHJOIN_FRAGMENTATION_FACTOR = new 
RangeDoubleValidator(HASHJOIN_FRAGMENTATION_FACTOR_KEY, 1.0, Double.MAX_VALUE, 
null);
+  public static final DoubleValidator HASHJOIN_FRAGMENTATION_FACTOR = new 
RangeDoubleValidator(HASHJOIN_FRAGMENTATION_FACTOR_KEY, 1.0, Double.MAX_VALUE,
+  new OptionDescription("Sets the Hash Join Memory Calculations; 
multiplies the internal estimates to account for fragmentation. Default is 
1.33"));
   public static final String HASHJOIN_NUM_ROWS_IN_BATCH_KEY = 
"exec.hashjoin.num_rows_in_batch";
-  public static final LongValidator HASHJOIN_NUM_ROWS_IN_BATCH_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_ROWS_IN_BATCH_KEY, 1, 65536, null);
+  public static final LongValidator HASHJOIN_NUM_ROWS_IN_BATCH_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_ROWS_IN_BATCH_KEY, 1, 65536,
+  new OptionDescription("Sets the number of rows in the internal batches 
for Hash Join operations. Default is 1024"));
   public static final String HASHJOIN_MAX_BATCHES_IN_MEMORY_KEY = 
"exec.hashjoin.max_batches_in_memory";
-  public static final LongValidator HASHJOIN_MAX_BATCHES_IN_MEMORY_VALIDATOR = 
new RangeLongValidator(HASHJOIN_MAX_BATCHES_IN_MEMORY_KEY, 0, 65536, null);
+  public static final LongValidator HASHJOIN_MAX_BATCHES_IN_MEMORY_VALIDATOR = 
new RangeLongValidator(HASHJOIN_MAX_BATCHES_IN_MEMORY_KEY, 0, 65536,
+  new OptionDescription("Sets the maximum number of batches allowed in 
memory before spilling is enforced for Hash Join operations; used for testing 
purposes."));
   public static final String HASHJOIN_NUM_PARTITIONS_KEY = 
"exec.hashjoin.num_partitions";
-  public static final LongValidator HASHJOIN_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_PARTITIONS_KEY, 1, 128, null); // 1 means - no 
spilling
+  public static final LongValidator HASHJOIN_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_PARTITIONS_KEY, 1, 128,
+  new OptionDescription("Sets the initial number of internal partitions 
for Hash Join operations. Default is 32. May reduce when memory is too small. 
Disables spilling if set to 1.")); // 1 means - no spilling
 
 Review comment:
   It's there in the description. The comment was previously there.. probably 
for new developers to know what the values mean.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] kkhatua commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
kkhatua commented on a change in pull request #1507: DRILL-6715: Update 
descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227486151
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
 ##
 @@ -480,6 +480,9 @@ private void generateOptionsDescriptionJSFile() throws 
IOException {
 OptionManager optionManager = 
this.drillbit.getContext().getOptionManager();
 OptionList publicOptions = optionManager.getPublicOptionList();
 List options = new ArrayList<>(publicOptions);
+//Add internal options
 
 Review comment:
   Since there is a WebUI interface for internal options that has the same 
layout, I thought it should be added. Part of this exercise is to make it 
easier for Doc team to also generate web-site content that is consistent.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] kkhatua commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
kkhatua commented on a change in pull request #1507: DRILL-6715: Update 
descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227485746
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -287,20 +303,21 @@ private ExecConstants() {
   PARQUET_WRITER_ENABLE_DICTIONARY_ENCODING,
   new OptionDescription("For internal use. Do not change."));
 
-  public static final String PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS
-  = "store.parquet.writer.use_primitive_types_for_decimals";
-  public static final OptionValidator 
PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS_VALIDATOR = new 
BooleanValidator(
-PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS, null);
+  public static final String PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS = 
"store.parquet.writer.use_primitive_types_for_decimals";
+  public static final OptionValidator 
PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS_VALIDATOR = new 
BooleanValidator(PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS,
+  new OptionDescription("Instructs the Parquet writer to convert decimal 
to primitive types whenever possible."));
 
-  public static final String PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS
-  = "store.parquet.writer.logical_type_for_decimals";
-  public static final OptionValidator 
PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS_VALIDATOR
-  = new 
EnumeratedStringValidator(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS, null, 
"fixed_len_byte_array", "binary");
+  public static final String PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS = 
"store.parquet.writer.logical_type_for_decimals";
+  public static final OptionValidator 
PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS_VALIDATOR = new 
EnumeratedStringValidator(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS,
+  new OptionDescription("Parquet writer logical type for decimal; 
supported types \'fixed_len_byte_array\' and \'binary\'"),
+  "fixed_len_byte_array", "binary");
 
+  // TODO - The below two options don't seem to be used in the Drill code base
 
 Review comment:
   I'll just add the description as 'Deprecated', since they are anyway not in 
use in the code anymore.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227350928
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseUrlFunction.java
 ##
 @@ -22,134 +22,238 @@
 import org.apache.drill.exec.expr.annotations.FunctionTemplate;
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
+import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
 import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_url",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+public class ParseUrlFunction {
 
-public class ParseUrlFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_url", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
+  public static class ParseUrl implements DrillSimpleFunc {
 
-@Param VarCharHolder input;
+@Param
+VarCharHolder in;
+@Output
+BaseWriter.ComplexWriter outWriter;
+@Inject
+DrillBuf outBuffer;
 
-@Output BaseWriter.ComplexWriter outWriter;
-
-@Inject DrillBuf outBuffer;
-
-public void setup() {}
+@Override
+public void setup() {
+}
 
+@Override
 public void eval() {
+  org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
urlMapWriter = outWriter.rootAsMap();
+
+  String urlString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
+  try {
+java.net.URL aURL = new java.net.URL(urlString);
+
+String protocol = aURL.getProtocol();
+String authority = aURL.getAuthority();
+String host = aURL.getHost();
+java.lang.Integer port = aURL.getPort();
+String path = aURL.getPath();
+String query = aURL.getQuery();
+String filename = aURL.getFile();
+String ref = aURL.getRef();
+
+org.apache.drill.exec.expr.holders.VarCharHolder rowHolder =
+new org.apache.drill.exec.expr.holders.VarCharHolder();
+
+urlMapWriter.start();
+
+byte[] protocolBytes = protocol.getBytes();
+outBuffer.reallocIfNeeded(protocolBytes.length);
+outBuffer.setBytes(0, protocolBytes);
+rowHolder.start = 0;
+rowHolder.end = protocolBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("protocol").write(rowHolder);
+
+byte[] authorityBytes = authority.getBytes();
+outBuffer.reallocIfNeeded(authorityBytes.length);
+outBuffer.setBytes(0, authorityBytes);
+rowHolder.start = 0;
+rowHolder.end = authorityBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("authority").write(rowHolder);
+
+byte[] hostBytes = host.getBytes();
+outBuffer.reallocIfNeeded(hostBytes.length);
+outBuffer.setBytes(0, hostBytes);
+rowHolder.start = 0;
+rowHolder.end = hostBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("host").write(rowHolder);
+
+byte[] pathBytes = path.getBytes();
+outBuffer.reallocIfNeeded(pathBytes.length);
+outBuffer.setBytes(0, pathBytes);
+rowHolder.start = 0;
+rowHolder.end = pathBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("path").write(rowHolder);
+
+if (query != null) {
+  byte[] queryBytes = query.getBytes();
+  outBuffer.reallocIfNeeded(queryBytes.length);
+  outBuffer.setBytes(0, queryBytes);
+  rowHolder.start = 0;
+  rowHolder.end = queryBytes.length;
+  rowHolder.buffer = outBuffer;
+  urlMapWriter.varChar("query").write(rowHolder);
+}
 
-org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
urlMapWriter = outWriter.rootAsMap();
-
-String urlString = 
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(input.start,
 input.end, input.buffer);
-
-try {
-java.net.URL aURL = new java.net.URL(urlString);
-
-String protocol = aURL.getProtocol();
-String authority = aURL.getAuthority();
-String host = aURL.getHost();
-java.lang.Integer port = aURL.getPort();
-String path = aURL.getPath();
-String query = aURL.getQuery();
-String filename = aURL.getFile();
-String ref = aURL.getRef();
-
-org.apache.drill.exec.expr.holders.VarCharHolder rowHolder = new 
org.apache.drill.exec.expr.holders.VarCharHolder();
-
-byte[] rowStringBytes = protocol.getBytes();
-
-outBuffer.reallocIfNe

[GitHub] arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: 
Update descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227357315
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -115,25 +116,35 @@ private ExecConstants() {
 
   // Hash Join Options
   public static final String HASHJOIN_HASHTABLE_CALC_TYPE_KEY = 
"exec.hashjoin.hash_table_calc_type";
-  public static final StringValidator HASHJOIN_HASHTABLE_CALC_TYPE = new 
StringValidator(HASHJOIN_HASHTABLE_CALC_TYPE_KEY, null);
+  public static final EnumeratedStringValidator HASHJOIN_HASHTABLE_CALC_TYPE = 
new EnumeratedStringValidator(HASHJOIN_HASHTABLE_CALC_TYPE_KEY,
+  new OptionDescription("Sets the Hash Join Memory Calculator type. 
Default is LEAN. This option also accepts CONSERVATIVE as a value."),
+  "LEAN", "CONSERVATIVE");
   public static final String HASHJOIN_SAFETY_FACTOR_KEY = 
"exec.hashjoin.safety_factor";
-  public static final DoubleValidator HASHJOIN_SAFETY_FACTOR = new 
RangeDoubleValidator(HASHJOIN_SAFETY_FACTOR_KEY, 1.0, Double.MAX_VALUE, null);
+  public static final DoubleValidator HASHJOIN_SAFETY_FACTOR = new 
RangeDoubleValidator(HASHJOIN_SAFETY_FACTOR_KEY, 1.0, Double.MAX_VALUE,
+  new OptionDescription("Sets the Hash Join Memory Calculation Safety; 
multiplies the internal size estimate. Default is 1.0"));
   public static final String HASHJOIN_HASH_DOUBLE_FACTOR_KEY = 
"exec.hashjoin.hash_double_factor";
-  public static final DoubleValidator HASHJOIN_HASH_DOUBLE_FACTOR = new 
RangeDoubleValidator(HASHJOIN_HASH_DOUBLE_FACTOR_KEY, 1.0, Double.MAX_VALUE, 
null);
+  public static final DoubleValidator HASHJOIN_HASH_DOUBLE_FACTOR = new 
RangeDoubleValidator(HASHJOIN_HASH_DOUBLE_FACTOR_KEY, 1.0, Double.MAX_VALUE,
+  new OptionDescription("Sets the Hash Join Memory Calculation; doubling 
factor for the Hash-Table. Default is 2.0"));
   public static final String HASHJOIN_FRAGMENTATION_FACTOR_KEY = 
"exec.hashjoin.fragmentation_factor";
-  public static final DoubleValidator HASHJOIN_FRAGMENTATION_FACTOR = new 
RangeDoubleValidator(HASHJOIN_FRAGMENTATION_FACTOR_KEY, 1.0, Double.MAX_VALUE, 
null);
+  public static final DoubleValidator HASHJOIN_FRAGMENTATION_FACTOR = new 
RangeDoubleValidator(HASHJOIN_FRAGMENTATION_FACTOR_KEY, 1.0, Double.MAX_VALUE,
+  new OptionDescription("Sets the Hash Join Memory Calculations; 
multiplies the internal estimates to account for fragmentation. Default is 
1.33"));
   public static final String HASHJOIN_NUM_ROWS_IN_BATCH_KEY = 
"exec.hashjoin.num_rows_in_batch";
-  public static final LongValidator HASHJOIN_NUM_ROWS_IN_BATCH_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_ROWS_IN_BATCH_KEY, 1, 65536, null);
+  public static final LongValidator HASHJOIN_NUM_ROWS_IN_BATCH_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_ROWS_IN_BATCH_KEY, 1, 65536,
+  new OptionDescription("Sets the number of rows in the internal batches 
for Hash Join operations. Default is 1024"));
   public static final String HASHJOIN_MAX_BATCHES_IN_MEMORY_KEY = 
"exec.hashjoin.max_batches_in_memory";
-  public static final LongValidator HASHJOIN_MAX_BATCHES_IN_MEMORY_VALIDATOR = 
new RangeLongValidator(HASHJOIN_MAX_BATCHES_IN_MEMORY_KEY, 0, 65536, null);
+  public static final LongValidator HASHJOIN_MAX_BATCHES_IN_MEMORY_VALIDATOR = 
new RangeLongValidator(HASHJOIN_MAX_BATCHES_IN_MEMORY_KEY, 0, 65536,
+  new OptionDescription("Sets the maximum number of batches allowed in 
memory before spilling is enforced for Hash Join operations; used for testing 
purposes."));
   public static final String HASHJOIN_NUM_PARTITIONS_KEY = 
"exec.hashjoin.num_partitions";
-  public static final LongValidator HASHJOIN_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_PARTITIONS_KEY, 1, 128, null); // 1 means - no 
spilling
+  public static final LongValidator HASHJOIN_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHJOIN_NUM_PARTITIONS_KEY, 1, 128,
+  new OptionDescription("Sets the initial number of internal partitions 
for Hash Join operations. Default is 32. May reduce when memory is too small. 
Disables spilling if set to 1.")); // 1 means - no spilling
 
 Review comment:
   Maybe add `// 1 means - no spilling` in the option description.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: 
Update descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227357507
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -145,23 +156,28 @@ private ExecConstants() {
 
   // Hash Aggregate Options
   public static final String HASHAGG_NUM_PARTITIONS_KEY = 
"exec.hashagg.num_partitions";
-  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128, null); // 1 means - no 
spilling
+  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128,
+  new OptionDescription("Sets the initial number of internal partitions 
for Hash Aggregates. Default is 32. May reduce when memory is too small. 
Disables spilling if set to 1.")); // 1 means - no spilling
 
 Review comment:
   Maybe add `// 1 means - no spilling` in the option description.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: 
Update descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227357819
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -145,23 +156,28 @@ private ExecConstants() {
 
   // Hash Aggregate Options
   public static final String HASHAGG_NUM_PARTITIONS_KEY = 
"exec.hashagg.num_partitions";
-  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128, null); // 1 means - no 
spilling
+  public static final LongValidator HASHAGG_NUM_PARTITIONS_VALIDATOR = new 
RangeLongValidator(HASHAGG_NUM_PARTITIONS_KEY, 1, 128,
+  new OptionDescription("Sets the initial number of internal partitions 
for Hash Aggregates. Default is 32. May reduce when memory is too small. 
Disables spilling if set to 1.")); // 1 means - no spilling
   public static final String HASHAGG_MAX_MEMORY_KEY = "exec.hashagg.mem_limit";
-  public static final LongValidator HASHAGG_MAX_MEMORY_VALIDATOR = new 
RangeLongValidator(HASHAGG_MAX_MEMORY_KEY, 0, Integer.MAX_VALUE, null);
+  public static final LongValidator HASHAGG_MAX_MEMORY_VALIDATOR = new 
RangeLongValidator(HASHAGG_MAX_MEMORY_KEY, 0, Integer.MAX_VALUE,
+  new OptionDescription("Enforces the value set as the maximum memory for 
the Hash Aggregates. Default is 0 (disabled)."));
 
 Review comment:
   The same as above, all these comments will be useful in the description.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1507: DRILL-6715: 
Update descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227356672
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java
 ##
 @@ -480,6 +480,9 @@ private void generateOptionsDescriptionJSFile() throws 
IOException {
 OptionManager optionManager = 
this.drillbit.getContext().getOptionManager();
 OptionList publicOptions = optionManager.getPublicOptionList();
 List options = new ArrayList<>(publicOptions);
+//Add internal options
 
 Review comment:
   Do we need internal options on Web UI?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227348845
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseQueryFunction.java
 ##
 @@ -23,65 +23,157 @@
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
 import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
+import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_query",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+/**
+ * The function splits up a query string and returns a map of the key-value 
pairs.
+ * For example, {@code parse_query('url?arg1=x&arg2=y')} will return:
+ * 
+ * {
+ *   "arg1": "x",
+ *   "arg2": "y"
+ * }
+ * 
+ */
+public class ParseQueryFunction {
 
-public class ParseQueryFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_query", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
+  public static class ParseQuery implements DrillSimpleFunc {
 
 @Param
-NullableVarCharHolder input;
-
+VarCharHolder in;
 @Output
 BaseWriter.ComplexWriter outWriter;
-
 @Inject
 DrillBuf outBuffer;
 
+@Override
 public void setup() {
 }
 
+@Override
 public void eval() {
-
-org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
queryMapWriter = outWriter.rootAsMap();
-
-String queryString = 
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(input.start,
 input.end, input.buffer);
-
-if( queryString.isEmpty() || queryString.equals("null")){
-queryString = "";
+  org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
mapWriter = outWriter.rootAsMap();
+
+  String queryString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
+  int questionMarkIndex = queryString.indexOf("?");
+  if (questionMarkIndex > -1) {
+// Leave query parameters only
 
 Review comment:
   Please add comment with better explanation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227352819
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseUrlFunction.java
 ##
 @@ -22,134 +22,238 @@
 import org.apache.drill.exec.expr.annotations.FunctionTemplate;
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
+import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
 import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_url",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+public class ParseUrlFunction {
 
-public class ParseUrlFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_url", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
+  public static class ParseUrl implements DrillSimpleFunc {
 
-@Param VarCharHolder input;
+@Param
+VarCharHolder in;
+@Output
+BaseWriter.ComplexWriter outWriter;
+@Inject
+DrillBuf outBuffer;
 
-@Output BaseWriter.ComplexWriter outWriter;
-
-@Inject DrillBuf outBuffer;
-
-public void setup() {}
+@Override
+public void setup() {
+}
 
+@Override
 public void eval() {
+  org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
urlMapWriter = outWriter.rootAsMap();
+
+  String urlString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
+  try {
+java.net.URL aURL = new java.net.URL(urlString);
+
+String protocol = aURL.getProtocol();
+String authority = aURL.getAuthority();
+String host = aURL.getHost();
+java.lang.Integer port = aURL.getPort();
+String path = aURL.getPath();
+String query = aURL.getQuery();
+String filename = aURL.getFile();
+String ref = aURL.getRef();
+
+org.apache.drill.exec.expr.holders.VarCharHolder rowHolder =
+new org.apache.drill.exec.expr.holders.VarCharHolder();
+
+urlMapWriter.start();
+
+byte[] protocolBytes = protocol.getBytes();
+outBuffer.reallocIfNeeded(protocolBytes.length);
+outBuffer.setBytes(0, protocolBytes);
+rowHolder.start = 0;
+rowHolder.end = protocolBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("protocol").write(rowHolder);
+
+byte[] authorityBytes = authority.getBytes();
+outBuffer.reallocIfNeeded(authorityBytes.length);
+outBuffer.setBytes(0, authorityBytes);
+rowHolder.start = 0;
+rowHolder.end = authorityBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("authority").write(rowHolder);
+
+byte[] hostBytes = host.getBytes();
+outBuffer.reallocIfNeeded(hostBytes.length);
+outBuffer.setBytes(0, hostBytes);
+rowHolder.start = 0;
+rowHolder.end = hostBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("host").write(rowHolder);
+
+byte[] pathBytes = path.getBytes();
+outBuffer.reallocIfNeeded(pathBytes.length);
+outBuffer.setBytes(0, pathBytes);
+rowHolder.start = 0;
+rowHolder.end = pathBytes.length;
+rowHolder.buffer = outBuffer;
+urlMapWriter.varChar("path").write(rowHolder);
+
+if (query != null) {
+  byte[] queryBytes = query.getBytes();
+  outBuffer.reallocIfNeeded(queryBytes.length);
+  outBuffer.setBytes(0, queryBytes);
+  rowHolder.start = 0;
+  rowHolder.end = queryBytes.length;
+  rowHolder.buffer = outBuffer;
+  urlMapWriter.varChar("query").write(rowHolder);
+}
 
-org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
urlMapWriter = outWriter.rootAsMap();
-
-String urlString = 
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(input.start,
 input.end, input.buffer);
-
-try {
-java.net.URL aURL = new java.net.URL(urlString);
-
-String protocol = aURL.getProtocol();
-String authority = aURL.getAuthority();
-String host = aURL.getHost();
-java.lang.Integer port = aURL.getPort();
-String path = aURL.getPath();
-String query = aURL.getQuery();
-String filename = aURL.getFile();
-String ref = aURL.getRef();
-
-org.apache.drill.exec.expr.holders.VarCharHolder rowHolder = new 
org.apache.drill.exec.expr.holders.VarCharHolder();
-
-byte[] rowStringBytes = protocol.getBytes();
-
-outBuffer.reallocIfNe

[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227348128
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseQueryFunction.java
 ##
 @@ -23,65 +23,157 @@
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
 import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
+import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_query",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+/**
+ * The function splits up a query string and returns a map of the key-value 
pairs.
+ * For example, {@code parse_query('url?arg1=x&arg2=y')} will return:
+ * 
+ * {
+ *   "arg1": "x",
+ *   "arg2": "y"
+ * }
+ * 
+ */
+public class ParseQueryFunction {
 
-public class ParseQueryFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_query", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
+  public static class ParseQuery implements DrillSimpleFunc {
 
 @Param
-NullableVarCharHolder input;
-
+VarCharHolder in;
 @Output
 BaseWriter.ComplexWriter outWriter;
-
 @Inject
 DrillBuf outBuffer;
 
+@Override
 public void setup() {
 }
 
+@Override
 public void eval() {
-
-org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
queryMapWriter = outWriter.rootAsMap();
-
-String queryString = 
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(input.start,
 input.end, input.buffer);
-
-if( queryString.isEmpty() || queryString.equals("null")){
-queryString = "";
+  org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
mapWriter = outWriter.rootAsMap();
+
+  String queryString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
+  int questionMarkIndex = queryString.indexOf("?");
 
 Review comment:
   Please add comment with explanation.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227335847
 
 

 ##
 File path: 
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestConvertFunctions.java
 ##
 @@ -241,6 +244,27 @@ public void testConvertFromJson_drill4693() throws 
Exception {
 
   }
 
+  @Test
+  public void testConvertFromJsonNullableInput() throws Exception {
+try (BufferedWriter writer = new BufferedWriter(new FileWriter(
 
 Review comment:
   Could you please add comment with file content?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227336044
 
 

 ##
 File path: 
exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestComplexTypeReader.java
 ##
 @@ -231,6 +232,19 @@ public void testComplexAndSimpleColumnSelection() throws 
Exception {
 test("select t.a.b, kvgen(t.a.c) from cp.`jsoninput/input4.json` t");
   }
 
+  @Test
+  public void testKVGenWithNullableInput() throws Exception {
+testBuilder()
 
 Review comment:
   Maybe generate the file? Since it's fairly small.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227330995
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java
 ##
 @@ -138,6 +138,9 @@
  * Indicates that a method's associated logical operation returns NULL if
  * either input is NULL, and therefore that the method must not be called
  * with null inputs.  (The calling framework must handle NULLs.)
+ *
+ * Not Supported for Functions with {@link Output} of type
+ * {@link 
org.apache.drill.exec.vector.complex.writer.BaseWriter.ComplexWriter}.
 
 Review comment:
   Aggregate functions are also no supported for this type of null handling 
strategy. Could you please mention this point as well?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227337409
 
 

 ##
 File path: 
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestParseFunctions.java
 ##
 @@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.fn.impl;
+
+import org.apache.drill.categories.SqlFunctionTest;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterFixtureBuilder;
+import org.apache.drill.test.ClusterTest;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+
+import static org.apache.drill.test.TestBuilder.mapOf;
+
+@Category(SqlFunctionTest.class)
+public class TestParseFunctions extends ClusterTest {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+generateDataSource();
+ClusterFixtureBuilder builder = ClusterFixture.builder(dirTestWatcher);
+startCluster(builder);
+  }
+
+  private static void generateDataSource() throws Exception {
+try (BufferedWriter writer = new BufferedWriter(new FileWriter(
+new File(dirTestWatcher.getRootDir(), "nullable_urls.json" {
+  String[] urls = 
{"\"ftp://somewhere.com:3190/someFile?a=12&b=someValue\"";, null, 
"\"http://someUrl?p1=v1&p2=v=2&\""};
 
 Review comment:
   Please add file content in the comment.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227345538
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MappifyUtility.java
 ##
 @@ -38,22 +38,29 @@
   public static final String fieldValue = "value";
 
   public static DrillBuf mappify(FieldReader reader, BaseWriter.ComplexWriter 
writer, DrillBuf buffer, String caller) {
-// Currently we expect single map as input
-if (DataMode.REPEATED == reader.getType().getMode() || 
!(reader.getType().getMinorType() == TypeProtos.MinorType.MAP)) {
+// Currently we expect single map or null as input
+if (reader.getType().getMode() == DataMode.REPEATED
+|| (reader.isSet() && reader.getType().getMinorType() != 
TypeProtos.MinorType.MAP)) {
   throw new DrillRuntimeException("kvgen function only supports Simple 
maps as input");
 }
 BaseWriter.ListWriter listWriter = writer.rootAsList();
 listWriter.startList();
 BaseWriter.MapWriter mapWriter = listWriter.map();
 
+if (!reader.isSet()) {
 
 Review comment:
   We can init `BaseWriter.MapWriter mapWriter = listWriter.map();` after null 
check.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227355195
 
 

 ##
 File path: 
exec/java-exec/src/test/java/org/apache/drill/exec/expr/fn/impl/TestStringFunctions.java
 ##
 @@ -1392,6 +1392,26 @@ public void testSplit() throws Exception {
 .run();
   }
 
+  @Test
+  public void testSplitWithNullInput() throws Exception {
+try (BufferedWriter writer = new BufferedWriter(new FileWriter(new 
File(dirTestWatcher.getRootDir(), "nullable_strings.json" {
 
 Review comment:
   Please add example of the file content.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227333486
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Mappify.java
 ##
 @@ -50,15 +50,18 @@
* scalar value fields
* value fields need to be of the same data type
*/
-  @FunctionTemplate(names = {"mappify", "kvgen"}, scope = 
FunctionTemplate.FunctionScope.SIMPLE, nulls = 
FunctionTemplate.NullHandling.NULL_IF_NULL, isRandom = true)
+  @FunctionTemplate(names = {"mappify", "kvgen"}, scope = 
FunctionTemplate.FunctionScope.SIMPLE, isRandom = true)
   public static class ConvertMapToKeyValuePairs implements DrillSimpleFunc {
+
 @Param  FieldReader reader;
 @Inject DrillBuf buffer;
 @Output ComplexWriter writer;
 
+@Override
 public void setup() {
 }
 
+@Override
 
 Review comment:
   Could you please check if reader can be null, for example, if we pass 
`cast(null as map)` constant etc.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227353968
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseUrlFunction.java
 ##
 @@ -22,134 +22,238 @@
 import org.apache.drill.exec.expr.annotations.FunctionTemplate;
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
+import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
 import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_url",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+public class ParseUrlFunction {
 
-public class ParseUrlFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_url", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
+  public static class ParseUrl implements DrillSimpleFunc {
 
-@Param VarCharHolder input;
+@Param
+VarCharHolder in;
+@Output
+BaseWriter.ComplexWriter outWriter;
+@Inject
+DrillBuf outBuffer;
 
-@Output BaseWriter.ComplexWriter outWriter;
-
-@Inject DrillBuf outBuffer;
-
-public void setup() {}
+@Override
+public void setup() {
+}
 
+@Override
 public void eval() {
+  org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
urlMapWriter = outWriter.rootAsMap();
+
+  String urlString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
+  try {
+java.net.URL aURL = new java.net.URL(urlString);
+
+String protocol = aURL.getProtocol();
 
 Review comment:
   Maybe create Map with key as property name (i.e. protocol), value as result 
of the appropriate getter, then go over this map and write results into map 
writer?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227352283
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseUrlFunction.java
 ##
 @@ -22,134 +22,238 @@
 import org.apache.drill.exec.expr.annotations.FunctionTemplate;
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
+import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
 import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_url",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+public class ParseUrlFunction {
 
-public class ParseUrlFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_url", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
 
 Review comment:
   Please add java doc with function input and output examples.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227331483
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
 ##
 @@ -65,7 +65,12 @@ public DrillAggFuncHolder(
   FunctionAttributes attributes,
   FunctionInitializer initializer) {
 super(attributes, initializer);
-checkArgument(attributes.getNullHandling() == NullHandling.INTERNAL, "An 
aggregation function is required to do its own null handling.");
+  }
+
+  @Override
+  protected void checkNullHandling(NullHandling nullHandling) {
 
 Review comment:
   1. Please move under public methods here and in another class.
   2.  Please update the message -> "... null handling strategy"


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227346773
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
 ##
 @@ -1372,11 +1371,11 @@ public void eval() {
 } // end of eval
   }
 
-  @FunctionTemplate(name = "split", scope = FunctionScope.SIMPLE, nulls = 
NullHandling.NULL_IF_NULL,
-outputWidthCalculatorType = 
OutputWidthCalculatorType.CUSTOM_FIXED_WIDTH_DEFUALT)
+  @FunctionTemplate(name = "split", scope = FunctionScope.SIMPLE, nulls = 
NullHandling.INTERNAL,
 
 Review comment:
   We can remove Internal handling declaration  since it's a default.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227349104
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ParseQueryFunction.java
 ##
 @@ -23,65 +23,157 @@
 import org.apache.drill.exec.expr.annotations.Output;
 import org.apache.drill.exec.expr.annotations.Param;
 import org.apache.drill.exec.expr.holders.NullableVarCharHolder;
+import org.apache.drill.exec.expr.holders.VarCharHolder;
 import org.apache.drill.exec.vector.complex.writer.BaseWriter;
 
 import javax.inject.Inject;
 
-@FunctionTemplate(
-name="parse_query",
-scope= FunctionTemplate.FunctionScope.SIMPLE,
-nulls = FunctionTemplate.NullHandling.NULL_IF_NULL
-)
+/**
+ * The function splits up a query string and returns a map of the key-value 
pairs.
+ * For example, {@code parse_query('url?arg1=x&arg2=y')} will return:
+ * 
+ * {
+ *   "arg1": "x",
+ *   "arg2": "y"
+ * }
+ * 
+ */
+public class ParseQueryFunction {
 
-public class ParseQueryFunction implements DrillSimpleFunc {
+  @FunctionTemplate(name = "parse_query", scope = 
FunctionTemplate.FunctionScope.SIMPLE)
+  public static class ParseQuery implements DrillSimpleFunc {
 
 @Param
-NullableVarCharHolder input;
-
+VarCharHolder in;
 @Output
 BaseWriter.ComplexWriter outWriter;
-
 @Inject
 DrillBuf outBuffer;
 
+@Override
 public void setup() {
 }
 
+@Override
 public void eval() {
-
-org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
queryMapWriter = outWriter.rootAsMap();
-
-String queryString = 
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(input.start,
 input.end, input.buffer);
-
-if( queryString.isEmpty() || queryString.equals("null")){
-queryString = "";
+  org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter 
mapWriter = outWriter.rootAsMap();
+
+  String queryString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
+  int questionMarkIndex = queryString.indexOf("?");
+  if (questionMarkIndex > -1) {
+// Leave query parameters only
+queryString = queryString.substring(questionMarkIndex + 1);
+  }
+
+  if (queryString.trim().isEmpty() || queryString.equals("null")) {
 
 Review comment:
   `equalsIgnoreCase`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
arina-ielchiieva commented on a change in pull request #1509: DRILL-6810: 
Disable NULL_IF_NULL NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509#discussion_r227347397
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java
 ##
 @@ -1396,20 +1395,66 @@ public void setup() {
 
 @Override
 public void eval() {
+  String inputString =
+  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(in.start,
 in.end, in.buffer);
   // Convert the iterable to an array as Janino will not handle generics.
-  Object[] tokens = 
com.google.common.collect.Iterables.toArray(splitter.split(
-  
org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(input.start,
 input.end, input.buffer)), String.class);
+  Object[] tokens = 
com.google.common.collect.Iterables.toArray(splitter.split(inputString), 
String.class);
   org.apache.drill.exec.vector.complex.writer.BaseWriter.ListWriter list = 
writer.rootAsList();
   list.startList();
-  for(int i = 0; i < tokens.length; i++ ) {
-final byte[] strBytes = 
((String)tokens[i]).getBytes(com.google.common.base.Charsets.UTF_8);
+  for (Object token : tokens) {
+final byte[] strBytes = ((String) 
token).getBytes(com.google.common.base.Charsets.UTF_8);
 buffer = buffer.reallocIfNeeded(strBytes.length);
 buffer.setBytes(0, strBytes);
 list.varChar().writeVarChar(0, strBytes.length, buffer);
   }
   list.endList();
 }
+  }
+
+  @FunctionTemplate(name = "split", scope = FunctionScope.SIMPLE, nulls = 
NullHandling.INTERNAL,
+  outputWidthCalculatorType = 
OutputWidthCalculatorType.CUSTOM_FIXED_WIDTH_DEFAULT)
+  public static class SplitNullableInput implements DrillSimpleFunc {
+@Param NullableVarCharHolder in;
+@Param VarCharHolder delimiter;
 
 Review comment:
   Did you check what will happen if delimiter is passed as null?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] vvysotskyi commented on a change in pull request #1507: DRILL-6715: Update descriptions for System Options table

2018-10-23 Thread GitBox
vvysotskyi commented on a change in pull request #1507: DRILL-6715: Update 
descriptions for System Options table
URL: https://github.com/apache/drill/pull/1507#discussion_r227318241
 
 

 ##
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##
 @@ -287,20 +303,21 @@ private ExecConstants() {
   PARQUET_WRITER_ENABLE_DICTIONARY_ENCODING,
   new OptionDescription("For internal use. Do not change."));
 
-  public static final String PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS
-  = "store.parquet.writer.use_primitive_types_for_decimals";
-  public static final OptionValidator 
PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS_VALIDATOR = new 
BooleanValidator(
-PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS, null);
+  public static final String PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS = 
"store.parquet.writer.use_primitive_types_for_decimals";
+  public static final OptionValidator 
PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS_VALIDATOR = new 
BooleanValidator(PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS,
+  new OptionDescription("Instructs the Parquet writer to convert decimal 
to primitive types whenever possible."));
 
-  public static final String PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS
-  = "store.parquet.writer.logical_type_for_decimals";
-  public static final OptionValidator 
PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS_VALIDATOR
-  = new 
EnumeratedStringValidator(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS, null, 
"fixed_len_byte_array", "binary");
+  public static final String PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS = 
"store.parquet.writer.logical_type_for_decimals";
+  public static final OptionValidator 
PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS_VALIDATOR = new 
EnumeratedStringValidator(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS,
+  new OptionDescription("Parquet writer logical type for decimal; 
supported types \'fixed_len_byte_array\' and \'binary\'"),
+  "fixed_len_byte_array", "binary");
 
+  // TODO - The below two options don't seem to be used in the Drill code base
 
 Review comment:
   Then it would be good to add `OptionDescription`s  for these options and 
point there that they are deprecated and will be removed.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] oleg-zinovev edited a comment on issue #1446: DRILL-6349: Drill JDBC driver fails on Java 1.9+ with NoClassDefFoundError: sun/misc/VM

2018-10-23 Thread GitBox
oleg-zinovev edited a comment on issue #1446: DRILL-6349: Drill JDBC driver 
fails on Java 1.9+ with NoClassDefFoundError: sun/misc/VM
URL: https://github.com/apache/drill/pull/1446#issuecomment-432165721
 
 
   > @oleg-zinovev, Is it make sence to move these two dependencies to the root 
pom?
   
   @vvysotskyi, it isn't make sence at all, so i remove this dependencies.
   
   I try to build Drill with fresh JDK 11 and fix new errors:
   1) JMockit 1.39 -> 1.43. JMockit had an issue with java 11 
(https://github.com/jmockit/jmockit1/issues/534), so I had to update this 
dependecy. Starting JMockit 1.42, "-javaagent" must be specified to run tests 
(http://jmockit.github.io/tutorial/Introduction.html#runningTests) and 
jmockit.integration.junit4.JMockit.class was removed. That's why i have to 
change this tests:
   - TestCastFunctions.java
   - TestConstantFolding.java
   - TestDateConversions.java
   - TestDateFunctions.java
   - TestExtendedTypes.java
   - TestUtf8SupportInQueryString.java
   - AvroFormatTest.java
   2) BoundsCheckingTest.java fails with IllegalAccessException. This was fixed 
by adding a call of setAccessible(true).
   3) JDKClassCompiler.java - Java 11 intoduces 'Nests' 
(http://openjdk.java.net/jeps/181), and resulting byte-code, generated by javac 
11 requires a "Nests" support from ASM visitors. Since Drill supports java 1.8+ 
i simply add a target and source version to javac call, to not have to change 
ASM visitors.
   4) hadoop 2.7.1 -> 2.7.4. Haboop org.apache.hadoop.util.Shell class in 
version 2.7.1 contains a Java version check:
   `System.getProperty("java.version").substring(0, 3).compareTo("1.7") >= 0;`
   But, for jdk 11 (at least for OpenJDK Ubuntu package), 
System.getProperty("java.version") return "11", and substring call fails. I had 
to update hadoop to minimal version, which not contains this call.
   5) TestImpersonationMetadata.java - in hadoop 2.7.4 error message was 
changed, so i slightly change a "expected" error to fit both new and old 
messages.
   6) maven-source-plugin fails to execute "jar-no-fork" on JDK 11, and i 
update it to the latest version. 
   
   Now on java 11 fails:
   1) HBase tests
   2) Hive test
   
   I will try to update a HBase and revalidate drill on JDK 9 and JDK 10 to the 
end of the week.
   
   P.S. I am sorry for possible mistakes because of my bad English


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] oleg-zinovev commented on issue #1446: DRILL-6349: Drill JDBC driver fails on Java 1.9+ with NoClassDefFoundError: sun/misc/VM

2018-10-23 Thread GitBox
oleg-zinovev commented on issue #1446: DRILL-6349: Drill JDBC driver fails on 
Java 1.9+ with NoClassDefFoundError: sun/misc/VM
URL: https://github.com/apache/drill/pull/1446#issuecomment-432165721
 
 
   '@oleg-zinovev, Is it make sence to move these two dependencies to the root 
pom?'
   
   @vvysotskyi, it isn't make sence at all, so i remove this dependencies.
   
   I try to build Drill with fresh JDK 11 and fix new errors:
   1) JMockit 1.39 -> 1.43. JMockit had an issue with java 11 
(https://github.com/jmockit/jmockit1/issues/534), so I had to update this 
dependecy. Starting JMockit 1.42, "-javaagent" must be specified to run tests 
(http://jmockit.github.io/tutorial/Introduction.html#runningTests) and 
jmockit.integration.junit4.JMockit.class was removed. That's why i have to 
change this tests:
   - TestCastFunctions.java
   - TestConstantFolding.java
   - TestDateConversions.java
   - TestDateFunctions.java
   - TestExtendedTypes.java
   - TestUtf8SupportInQueryString.java
   - AvroFormatTest.java
   2) BoundsCheckingTest.java fails with IllegalAccessException. This was fixed 
by adding a call of setAccessible(true).
   3) JDKClassCompiler.java - Java 11 intoduces 'Nests' 
(http://openjdk.java.net/jeps/181), and resulting byte-code, generated by javac 
11 requires a "Nests" support from ASM visitors. Since Drill supports java 1.8+ 
i simply add a target and source version to javac call, to not have to change 
ASM visitors.
   4) hadoop 2.7.1 -> 2.7.4. Haboop org.apache.hadoop.util.Shell class in 
version 2.7.1 contains a Java version check:
   "System.getProperty("java.version").substring(0, 3).compareTo("1.7") >= 0;"
   But, for jdk 11 (at least for OpenJDK Ubuntu package), 
System.getProperty("java.version") return "11", and substring call fails. I had 
to update hadoop to minimal version, which not contains this call.
   5) TestImpersonationMetadata.java - in hadoop 2.7.4 error message was 
changed, so i slightly change a "expected" error to fit both new and old 
messages.
   6) maven-source-plugin fails to execute "jar-no-fork" on JDK 11, and i 
update it to the latest version. 
   
   Now on java 11 fails:
   1) HBase tests
   2) Hive test
   
   I will try to update a HBase and revalidate drill on JDK 9 and JDK 10 to the 
end of the week.
   
   P.S. I am sorry for possible mistakes because of my bad English


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] KazydubB opened a new pull request #1509: DRILL-6810: Disable NULL_IF_NULL NullHandling for functions with Comp…

2018-10-23 Thread GitBox
KazydubB opened a new pull request #1509: DRILL-6810: Disable NULL_IF_NULL 
NullHandling for functions with Comp…
URL: https://github.com/apache/drill/pull/1509
 
 
   …lexWriter


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (DRILL-6810) Disable NULL_IF_NULL NullHandling for functions with ComplexWriter

2018-10-23 Thread Bohdan Kazydub (JIRA)
Bohdan Kazydub created DRILL-6810:
-

 Summary: Disable NULL_IF_NULL NullHandling for functions with 
ComplexWriter
 Key: DRILL-6810
 URL: https://issues.apache.org/jira/browse/DRILL-6810
 Project: Apache Drill
  Issue Type: Bug
Reporter: Bohdan Kazydub
Assignee: Bohdan Kazydub


Currently NullHandling.NULL_IF_NULL is allowed for UDFs with @Output of type 
org.apache.drill.exec.vector.complex.writer.BaseWriter.ComplexWriter but no 
null handling is performed for the kind of functions which leads to confusion. 
The problem is ComplexWriter holds list/map values and Drill does not yet 
support NULL values for the types (there is an issue to allow null maps/lists 
in [DRILL-4824|https://issues.apache.org/jira/browse/DRILL-4824]).
For such functions support for NULL_IF_NULL will be disabled, as it is done for 
aggregate functions, and NullHandling.INTERNAL should be used instead.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL functions with constant values

2018-10-23 Thread GitBox
lushuifeng commented on issue #1481: DRILL-6763: Codegen optimization of SQL 
functions with constant values
URL: https://github.com/apache/drill/pull/1481#issuecomment-432132394
 
 
   Thank you for your review and comments.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services