[GitHub] ilooner closed pull request #1508: DRILL-6804: Simplify usage of OperatorPhase in HashAgg.
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.
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
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
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
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
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
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
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…
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
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
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
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
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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…
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
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
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
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…
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
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
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