Re: Which code compiler is better

2017-08-11 Thread weijie tong
@paul why not set the default code generation be plain java? The current
default ASM merge method will have some unpredictable scalar replacement
errors which we ever suffered before. This will give users trustless feal.
What the merging does will also be done by the JIT if  we choose the plain
java to extend the template.


On Fri, 11 Aug 2017 at 5:52 PM weijie tong  wrote:

> @chunhui  we just adjust different compiler options ,the generating code
> strategy does not affected by the compiler option.  so I think the
> different result just reflects the compiler's performance.
>
> On Wed, Aug 2, 2017 at 1:45 AM, Chunhui Shi  wrote:
>
>>
>> Correct my previous response:
>>
>> In DRILL-4778, JDK was faster in compilation but generated slower code.
>> Janino was slower in compilation and generate faster code. Your JIRA did
>> not mention how was the performance when running generated code. You may
>> want to test this aspect as well.
>>
>>
>> From: weijie tong 
>> Sent: Sunday, July 30, 2017 6:10:12 AM
>> To: dev@drill.apache.org
>> Subject: Which code compiler is better
>>
>> The compile process is long when we have 20 sum or avg expression and the
>> compiler is janino. But if we change the compiler to jdk,we gain lower
>> compile process time. It seems jdk compiler is better .If that's tue,why
>> not let jdk be the default one?
>>
>
>


[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

2017-08-11 Thread Ben-Zvi
Github user Ben-Zvi commented on a diff in the pull request:

https://github.com/apache/drill/pull/901#discussion_r132806738
  
--- Diff: 
exec/vector/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---
@@ -133,5 +134,21 @@ public static boolean checkBufRefs(final ValueVector 
vv) {
   public BufferAllocator getAllocator() {
 return allocator;
   }
+
+  public static void fillBitsVector(UInt1Vector bits, int valueCount) {
+
+// Create a new bits vector, all values non-null
+
+bits.allocateNew(valueCount);
+UInt1Vector.Mutator bitsMutator = bits.getMutator();
+for (int i = 0; i < valueCount; i++) {
+  bitsMutator.set(i, 1);
+}
+  }
--- End diff --

And need to add:

bitsMutator.setValueCount(valueCount);




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

2017-08-11 Thread Ben-Zvi
Github user Ben-Zvi commented on a diff in the pull request:

https://github.com/apache/drill/pull/901#discussion_r132806694
  
--- Diff: 
exec/vector/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---
@@ -133,5 +134,21 @@ public static boolean checkBufRefs(final ValueVector 
vv) {
   public BufferAllocator getAllocator() {
 return allocator;
   }
+
+  public static void fillBitsVector(UInt1Vector bits, int valueCount) {
+
+// Create a new bits vector, all values non-null
+
+bits.allocateNew(valueCount);
+UInt1Vector.Mutator bitsMutator = bits.getMutator();
+for (int i = 0; i < valueCount; i++) {
+  bitsMutator.set(i, 1);
+}
--- End diff --

This loop may be a bit expensive; Is there a way to mark the whole bitmap 
with '1's "at once" ?  A la "memset()".  
For example, keep a constant bitmap (of max length - 64K) someplace, and 
here just dup/copy it whole.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #904: DRILL-5717: let some date time test cases be Local indepen...

2017-08-11 Thread weijietong
Github user weijietong commented on the issue:

https://github.com/apache/drill/pull/904
  
Yes, thanks for the notice ,have noticed those cases before ,the work is in 
process.some of that will be part of this PR. Others like sql_to_timestamp 
function need to discuss whether it should have a Local parameter or not set 
the specific UTC timezone when get the date.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Error message: Memory was leaked by query

2017-08-11 Thread Muhammad Gelbana
I'm trying to run the following query

*​​*
*SELECT op.​​platform, op.name , op.paymentType,
ck.posDiscountName, sum(op.amount) amt FROM `dfs`.`/​path_to_parquet` op,
`dfs`.`​path_to_parquet​2​` ck WHERE ck.id  = op.check_id
GROUP BY op.platform, op.name , op.paymentType,
ck.posDiscountName LIMIT 2147483647*

I also tried the same query without the LIMIT clause
 but it still fails for
the same reason.
​​
I'm facing the following exception in the logs and I'm not sure how to
resolve it.

Suppressed: java.lang.IllegalStateException: Memory was leaked by query.
> Memory leaked: (4194304)
> Allocator(op:0:0:0:Screen) 100/4194304/12582912/100
> (res/actual/peak/limit)
> at
> org.apache.drill.exec.memory.BaseAllocator.close(BaseAllocator.java:492)
> at
> org.apache.drill.exec.ops.OperatorContextImpl.close(OperatorContextImpl.java:141)
> at
> org.apache.drill.exec.ops.FragmentContext.suppressingClose(FragmentContext.java:422)
> at
> org.apache.drill.exec.ops.FragmentContext.close(FragmentContext.java:411)
> at
> org.apache.drill.exec.work.fragment.FragmentExecutor.closeOutResources(FragmentExecutor.java:318)
> at
> org.apache.drill.exec.work.fragment.FragmentExecutor.cleanup(FragmentExecutor.java:155)
> at
> org.apache.drill.exec.work.fragment.FragmentExecutor.run(FragmentExecutor.java:262)
> at
> org.apache.drill.common.SelfCleaningRunnable.run(SelfCleaningRunnable.java:38)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> ... 1 more
> Suppressed: java.lang.IllegalStateException: Memory was leaked by
> query. Memory leaked: (4194304)
> Allocator(frag:0:0) 300/4194304/1511949440/300
> (res/actual/peak/limit)
> at
> org.apache.drill.exec.memory.BaseAllocator.close(BaseAllocator.java:492)
> at
> org.apache.drill.exec.ops.FragmentContext.suppressingClose(FragmentContext.java:422)
> at
> org.apache.drill.exec.ops.FragmentContext.close(FragmentContext.java:416)
> at
> org.apache.drill.exec.work.fragment.FragmentExecutor.closeOutResources(FragmentExecutor.java:318)
> at
> org.apache.drill.exec.work.fragment.FragmentExecutor.cleanup(FragmentExecutor.java:155)
> at
> org.apache.drill.exec.work.fragment.FragmentExecutor.run(FragmentExecutor.java:262)
> at
> org.apache.drill.common.SelfCleaningRunnable.run(SelfCleaningRunnable.java:38)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> ... 1 more


The UI is showing the following error
*org.apache.drill.common.exceptions.UserException: CONNECTION ERROR:
Connection /1.1.1.1:40834  <--> Gelbana/1.1.1.1:31010
 (user client) closed unexpectedly. Drillbit down?
[Error Id: 268bc3a7-114f-4681-984c-05d143f7ebd9 ]*

I understand that this bug has been fixed in 1.9
, which is the version
I'm using. I did what the comments suggested which is to tell Drill to use
a tmp directory that has enough space, so I set the JVM option
*java.io.tmpdir* to /home/mgelbana/server*/temp/* which has over 100GB of
free space, and modified the drill-override.conf file to have the following

tmp: {
> directories: ["/home/mgelbana/server/temp/"],
> filesystem: "file:///"
>   },
>   sort: {
> external: {
>   spill: {
> batch.size : 4000,
> group.size : 100,
> threshold : 200,
> directories : [ "/home/mgelbana/server/temp/spill" ],
> fs : "file:///"
>   }
> }
>   }


I'm running a single Drillbit on a single machine with 25 GB of heap memory
and a 100 GB of direct memory. The machine has 48 cores (i.e. the output of
*nproc* on linux)

*planner.width.max_per_node = 40*
*planner.memory.max_query_memory_per_node = 8589934592 (8 GB)*

That's the plan of the query is

00-00Screen : rowType = RecordType(ANY platform, ANY name, ANY
> paymentType, ANY posDiscountName, ANY amt): rowcount = 2.147483647E9,
> cumulative cost = {7.00120818116E9 rows, 3.700395926736E10 cpu, 0.0 io,
> 8.6479703758848E12 network, 1.444996068119E10 memory}, id = 24229
> 00-01  Project(platform=[$0], name=[$1], paymentType=[$2],
> posDiscountName=[$3], amt=[$4]) : rowType = RecordType(ANY platform, ANY
> name, ANY paymentType, ANY posDiscountName, ANY amt): rowcount =
> 2.147483647E9, cumulative cost = {6.78645981646E9 rows,
> 3.678921090266E10 cpu, 0.0 io, 

[GitHub] drill issue #858: DRILL-3640: Support JDBC Statement.setQueryTimeout(int)

2017-08-11 Thread parthchandra
Github user parthchandra commented on the issue:

https://github.com/apache/drill/pull/858
  
Been looking at this and the first thing that occurs to me is that we are 
not too clear about what the timeout means in the context of ResultSet. The API 
specification is rather silent on that topic. 
The only reference I could find to this question is this one: 
http://mail-archives.apache.org/mod_mbox/db-derby-dev/200504.mbox/%3c426909ba.1060...@sun.com%3E
We have the same choices:
 

> 1. setQueryTimeout() only affects Statement.execute()
> 2.  setQueryTimeout() affects Statement.execute() and 
ResultSet.next(),starting from zero for each invocation
> 3.  setQueryTimeout() affects Statement.execute() and 
ResultSet.next(),accumulating time spent in each invocation

My own inclination was to select #2 as the appropriate behavior. In fact 
that is what I assumed before I looked at the code. Laurent's suggestion to 
implement the timeout in DrillCursor provides this behavior and is a little bit 
easier to implement.

OTOH, Kunal has chosen #3 as the right behavior. MySQL implements this 
behavior, BTW, so it is not going to be a surprise to end users. And he has 
already done the work. 

I'm +0 on this so far. Let me see if I can get a quick prototype to test 
things out.  



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #900: DRILL-4735: ConvertCountToDirectScan rule enhancements

2017-08-11 Thread jinfengni
Github user jinfengni commented on the issue:

https://github.com/apache/drill/pull/900
  
+1

LGTM. Thanks for the PR!



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132802715
  
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -93,6 +93,13 @@ drill.exec: {
   credentials: true
 }
   },
+  # Below SSL parameters need to be set for custom transport layer 
settings.
+  ssl: {
+keyStorePath: "/keystore.file",
+keyStorePassword: "ks_passwd",
+trustStorePath: "/truststore.file",
+trustStorePassword: "ts_passwd"
--- End diff --

Comments as suggested in previous comment?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #874: DRILL-5663: Drillbit fails to start when only keystore pat...

2017-08-11 Thread sindhurirayavaram
Github user sindhurirayavaram commented on the issue:

https://github.com/apache/drill/pull/874
  
Changes made and added another commit!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


direct.used on the Metrics page exceeded planner.memory.max_query_memory_per_node while running a single query

2017-08-11 Thread Muhammad Gelbana
Sorry for the long subject !

I'm running a single query on a single node Drill setup.

I assumed that setting the *planner.memory.max_query_memory_per_node* property
controls the max amount of memory (in bytes) for each running on a single
node. Which means that in my setup, the *direct.used* metric in the metrics
page should never exceed that value in my case.

But it did and drastically. I assigned *34359738368* (32 GB) to the
*planner.memory.max_query_memory_per_node* option but while monitoring the
*direct.used* metric, I found that it reached *51640484458* (~48 GB).

What did I mistakenly do\interpret ?

Thanks,
Gelbana
​​


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132800512
  
--- Diff: 
exec/java-exec/src/test/java/org/apache/drill/exec/TestSSLConfig.java ---
@@ -0,0 +1,108 @@
+/**
+ * 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;
+
+
+import org.apache.drill.common.exceptions.DrillException;
+import org.apache.drill.test.ConfigBuilder;
+import org.junit.Test;
+import static junit.framework.TestCase.fail;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestSSLConfig {
+
+  @Test
+  public void testMissingKeystorePath() throws Exception {
+
+ConfigBuilder config = new ConfigBuilder();
+config.put(ExecConstants.HTTP_KEYSTORE_PATH, "");
+config.put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "root");
+try {
+  SSLConfig sslv = new SSLConfig(config.build());
+  fail();
+  //Expected
+} catch (Exception e) {
+  assertTrue(e instanceof DrillException);
+}
+  }
+
+  @Test
+  public void testMissingKeystorePassword() throws Exception {
+
+ConfigBuilder config = new ConfigBuilder();
+config.put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+config.put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "");
+try {
+  SSLConfig sslv = new SSLConfig(config.build());
+  fail();
+  //Expected
+} catch (Exception e) {
+  assertTrue(e instanceof DrillException);
+}
+  }
+
+  @Test
+  public void testForKeystoreConfig() throws Exception {
+
+ConfigBuilder config = new ConfigBuilder();
+config.put(ExecConstants.HTTP_KEYSTORE_PATH, "/root");
+config.put(ExecConstants.HTTP_KEYSTORE_PASSWORD, "root");
+try {
+  SSLConfig sslv = new SSLConfig(config.build());
+  assertEquals("/root", sslv.getKeyStorePath());
+  assertEquals("root", sslv.getKeyStorePassword());
+} catch (Exception e) {
+  fail();
+
+}
+  }
+
+  @Test
+  public void testForBackwardCompatability() throws Exception {
+
+ConfigBuilder config = new ConfigBuilder();
+config.put("javax.net.ssl.keyStore", "/root");
+config.put("javax.net.ssl.keyStorePassword", "root");
+SSLConfig sslv = new SSLConfig(config.build());
+assertEquals("/root",sslv.getKeyStorePath());
+assertEquals("root", sslv.getKeyStorePassword());
+  }
+
+  @Test
+  public void testForTrustStore() throws Exception {
+
+ConfigBuilder config = new ConfigBuilder();
+config.put(ExecConstants.HTTP_TRUSTSTORE_PATH, "/root");
+config.put(ExecConstants.HTTP_TRUSTSTORE_PASSWORD, "root");
+SSLConfig sslv = new SSLConfig(config.build());
+assertEquals(true, sslv.hasTrustStorePath());
+assertEquals(true,sslv.hasTrustStorePassword());
+assertEquals("/root",sslv.getTrustStorePath());
+assertEquals("root",sslv.getTrustStorePassword());
+  }
+}
--- End diff --

Thanks for the tests!

Maybe delete the extra lines at the end of the file...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132800461
  
--- Diff: exec/java-exec/src/main/resources/drill-module.conf ---
@@ -52,6 +52,14 @@ drill.client: {
 drill.tmp-dir: "/tmp"
 drill.tmp-dir: ${?DRILL_TMP_DIR}
 
+javax.net.ssl :
+ {
+ keyStore = "",
+ keyStorePassword = "",
+ trustStore = "",
+ trustStorePassword = ""
+ },
--- End diff --

{code}
javax.net.ssl: {
  keyStore = "",
{code}

That is, adjust formatting of the header line and indent the values.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132800092
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -139,7 +142,13 @@ public void start() throws Exception {
 
 final ServerConnector serverConnector;
 if (config.getBoolean(ExecConstants.HTTP_ENABLE_SSL)) {
-  serverConnector = createHttpsConnector();
+  try {
+serverConnector = createHttpsConnector();
+  }
+  catch(DrillException e){
--- End diff --

{code}
catch (DrillbitException e) {
{code}
(note spaces)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132799898
  
--- Diff: distribution/src/resources/drill-override-example.conf ---
@@ -93,6 +93,13 @@ drill.exec: {
   credentials: true
 }
   },
+  # Below SSL parameters need to be set for custom transport layer 
settings.
+  ssl{
+keyStorePath: "/keystore.file",
+keyStorePassword: "ks_passwd",
+trustStorePath: "/truststore.file",
+trustStorePassword: "ts_passwd"
--- End diff --

ssl{ --> ssl: {


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132800101
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -139,7 +142,13 @@ public void start() throws Exception {
 
 final ServerConnector serverConnector;
 if (config.getBoolean(ExecConstants.HTTP_ENABLE_SSL)) {
-  serverConnector = createHttpsConnector();
+  try {
+serverConnector = createHttpsConnector();
+  }
+  catch(DrillException e){
+throw new DrillbitStartupException(e.getMessage(),e);
--- End diff --

Space after comma.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132800051
  
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/SSLConfig.java 
---
@@ -0,0 +1,77 @@
+/*
+ * 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;
+
+import com.typesafe.config.Config;
+import org.apache.drill.common.exceptions.DrillException;
+
+public class SSLConfig {
+
+  private final String keystorePath;
+
+  private final String keystorePassword;
+
+  private final String truststorePath;
+
+  private final String truststorePassword;
+
+
+  public SSLConfig(Config config) throws DrillException {
+
+keystorePath = 
config.getString(ExecConstants.HTTP_KEYSTORE_PATH).trim();
+
+keystorePassword = 
config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD).trim();
+
+truststorePath = 
config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH).trim();
+
+truststorePassword = 
config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD).trim();
+
+/*If keystorePath or keystorePassword is provided in the configuration 
file use that*/
+if (!keystorePath.isEmpty() || !keystorePassword.isEmpty()) {
+  if (keystorePath.isEmpty()) {
+throw new DrillException(" *.ssl.keyStorePath in the configuration 
file is empty, but *.ssl.keyStorePassword is set");
+  }
+  else if (keystorePassword.isEmpty()){
+throw new DrillException(" *.ssl.keyStorePassword in the 
configuration file is empty, but *.ssl.keyStorePath is set ");
+  }
+
+}
+  }
+
+  public boolean isSslValid() { return !keystorePath.isEmpty() && 
!keystorePassword.isEmpty(); }
+
+  public String getKeyStorePath() {
+return keystorePath;
+  }
+
+  public String getKeyStorePassword() {
+return keystorePassword;
+  }
+
+  public boolean hasTrustStorePath() { return !truststorePath.isEmpty(); }
+
+  public boolean hasTrustStorePassword() {return 
!truststorePassword.isEmpty(); }
+
+  public String getTrustStorePath() {
+return truststorePath;
+  }
+
+  public String getTrustStorePassword() {
+return truststorePassword;
+  }
--- End diff --

Very minor suggestion: these simple returns can also be one-liners like the 
"has" methods.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #874: DRILL-5663: Drillbit fails to start when only keyst...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request:

https://github.com/apache/drill/pull/874#discussion_r132800128
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
@@ -263,18 +272,17 @@ private ServerConnector createHttpsConnector() throws 
Exception {
 logger.info("Setting up HTTPS connector for web server");
 
 final SslContextFactory sslContextFactory = new SslContextFactory();
+SSLConfig ssl = new SSLConfig(config);
 
-if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
-
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
+if(ssl.isSslValid()){
--- End diff --

{code}
if (...) {
{code}


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #902: DRILL-5714: Fix NPE when mapr-db plugin is used in table f...

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/902
  
Thanks for the explanation. Looks good.
+1 (again)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #860: DRILL-5601: Rollup of external sort fixes and improvements

2017-08-11 Thread paul-rogers
Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/860
  
Rebased onto master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #906: DRILL-5546: Handle schema change exception failure ...

2017-08-11 Thread jinfengni
GitHub user jinfengni opened a pull request:

https://github.com/apache/drill/pull/906

DRILL-5546: Handle schema change exception failure caused by empty in…

…put or empty batche.

1. Modify ScanBatch's logic when it iterates list of RecordReader.
   1) Skip RecordReader if it returns 0 row && present same schema. A new 
schema (by calling Mutator.isNewSchema() ) means either a new top level field 
is added, or a field in a nested field is added, or an existing field type is 
changed.
   2) Implicit columns are added and populated only when the input is not 
empty, i.e. the batch contains > 0 row or rowCount == 0 && new schema.
   3) ScanBatch will return NONE directly (called as "fast NONE"), if all 
its RecordReaders haver empty input and thus are skipped, in stead of returing 
OK_NEW_SCHEMA first.

2. Modify IteratorValidatorBatchIterator to allow
   1) fast NONE ( before seeing a OK_NEW_SCHEMA)
   2) batch with empty list of columns.

2. Modify JsonRecordReader when it get 0 row. Do not insert a nullable-int 
column for 0 row input. Together with ScanBatch, Drill will skip empty json 
files.

3. Modify binary operators such as join, union to handle fast none for 
either one side or both sides. Abstract the logic in AbstractBinaryRecordBatch, 
except for MergeJoin as its implementation is quite different from others.

4. Fix and refactor union all operator.
  1) Correct union operator hanndling 0 input rows. Previously, it will 
ignore inputs with 0 row and put nullable-int into output schema, which causes 
various of schema change issue in down-stream operator. The new behavior is to 
take schema with 0 into account
  in determining the output schema, in the same way with > 0 input rows. By 
doing that, we ensure Union operator will not behave like a schema-lossy 
operator.
  2) Add a UnionInputIterator to simplify the logic to iterate the 
left/right inputs, removing significant chunk of duplicate codes in previous 
implementation.
  The new union all operator reduces the code size into half, comparing the 
old one.

5. Introduce UntypedNullVector to handle convertFromJson() function, when 
the input batch contains 0 row.
  Problem: The function convertFromJSon() is different from other regular 
functions in that it only knows the output schema after evaluation is 
performed. When input has 0 row, Drill essentially does not have
  a way to know the output type, and previously will assume Map type. That 
works under the assumption other operators like Union would ignore batch with 0 
row, which is no longer
  the case in the current implementation.
  Solution: Use MinorType.NULL at the output type for convertFromJSON() 
when input contains 0 row. The new UntypedNullVector is used to represent a 
column with MinorType.NULL.

6. HBaseGroupScan convert star column into list of row_key and column 
family. HBaseRecordReader should reject column star since it expectes star has 
been converted somewhere else.
  In HBase a column family always has map type, and a non-rowkey column 
always has nullable varbinary type, this ensures that HBaseRecordReader across 
different HBase regions will have the same top level schema, even if the region 
is
  empty or prune all the rows due to filter pushdown optimization. In other 
words, we will not see different top level schema from different 
HBaseRecordReader for the same table.
  However, such change will not be able to handle hard schema change : c1 
exists in cf1 in one region, but not in another region. Further work is 
required to handle hard schema change.

7. Modify scan cost estimation when the query involves * column. This is to 
remove the planning randomness since previously two different operators could 
have same cost.

8. Add a new flag 'outputProj' to Project operator, to indicate if Project 
is for the query's final output. Such Project is added by TopProjectVisitor, to 
handle fast NONE when all the inputs to the query are empty
and are skipped.
  1) column star is replaced with empty list
  2) regular column reference is replaced with nullable-int column
  3) An expression will go through ExpressionTreeMaterializer, and use the 
type of materialized expression as the output type
  4) Return an OK_NEW_SCHEMA with the schema using the above logic, then 
return a NONE to down-stream operator.

9. Add unit test to test operators handling empty input.

10. Add unit test to test query when inputs are all empty.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jinfengni/incubator-drill DRILL-5546

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/906.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in 

[GitHub] drill issue #895: DRILL-5704: Improve error message on client side when quer...

2017-08-11 Thread parthchandra
Github user parthchandra commented on the issue:

https://github.com/apache/drill/pull/895
  
+1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #904: DRILL-5717: let some date time test cases be Local indepen...

2017-08-11 Thread vvysotskyi
Github user vvysotskyi commented on the issue:

https://github.com/apache/drill/pull/904
  
Some tests from `TestDateFunctions`, `TestDateConversions` and 
`TestConstantFolding` classes also fails with some other locales (I checked it 
for `zh_TW` locale). 
I think they should also be fixed in the border of this PR.
Are there any other tests that depend on the locale?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill pull request #905: DRILL-1162: Fix OOM for hash join operator when the...

2017-08-11 Thread vvysotskyi
GitHub user vvysotskyi opened a pull request:

https://github.com/apache/drill/pull/905

DRILL-1162: Fix OOM for hash join operator when the right input has a much 
larger actual number of rows than the left one

Please see the problem description in 
[DRILL-1162](https://issues.apache.org/jira/browse/DRILL-1162).

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vvysotskyi/drill DRILL-1162

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/905.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #905


commit de9a32997774acf34d2cb42f534decac5fd75cb5
Author: Volodymyr Vysotskyi 
Date:   2017-08-09T21:21:48Z

DRILL-1162: Fix OOM for hash join operator when the right input has a much 
larger actual number of rows than the left one




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: Which code compiler is better

2017-08-11 Thread weijie tong
@chunhui  we just adjust different compiler options ,the generating code
strategy does not affected by the compiler option.  so I think the
different result just reflects the compiler's performance.

On Wed, Aug 2, 2017 at 1:45 AM, Chunhui Shi  wrote:

>
> Correct my previous response:
>
> In DRILL-4778, JDK was faster in compilation but generated slower code.
> Janino was slower in compilation and generate faster code. Your JIRA did
> not mention how was the performance when running generated code. You may
> want to test this aspect as well.
>
>
> From: weijie tong 
> Sent: Sunday, July 30, 2017 6:10:12 AM
> To: dev@drill.apache.org
> Subject: Which code compiler is better
>
> The compile process is long when we have 20 sum or avg expression and the
> compiler is janino. But if we change the compiler to jdk,we gain lower
> compile process time. It seems jdk compiler is better .If that's tue,why
> not let jdk be the default one?
>


[GitHub] drill pull request #904: DRILL-5717: let some date time test cases be Local ...

2017-08-11 Thread weijietong
GitHub user weijietong opened a pull request:

https://github.com/apache/drill/pull/904

DRILL-5717: let some date time test cases be Local independent

Some date time test cases is US Local .This will cause developers from 
other areas to fail. Fix is to specify the explicit Local to the test cases. 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/weijietong/drill DRILL-5717

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/904.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #904


commit b44f780a948c4a0898e7cee042c0590f0713f780
Author: weijietong 
Date:   2017-06-08T08:03:46Z

Merge pull request #1 from apache/master

sync

commit d045c757c80a759b435479cc89f33c749fc16ac2
Author: weijie.tong 
Date:   2017-08-11T08:01:36Z

Merge branch 'master' of github.com:weijietong/drill

commit 906eef175f6751663e58f36519b325f5a4e6dfea
Author: weijie.tong 
Date:   2017-08-11T08:14:54Z

let some test cases be Local independent




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[jira] [Created] (DRILL-5717) date time test cases is not Local independent

2017-08-11 Thread weijie.tong (JIRA)
weijie.tong created DRILL-5717:
--

 Summary: date time test cases is not Local independent
 Key: DRILL-5717
 URL: https://issues.apache.org/jira/browse/DRILL-5717
 Project: Apache Drill
  Issue Type: Bug
  Components: Tools, Build & Test
Affects Versions: 1.11.0, 1.9.0
Reporter: weijie.tong


Some date time test cases like  JodaDateValidatorTest  is not Local independent 
.This will cause other Local's users's test phase to fail. We should let these 
test cases to be Local env independent.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)