[GitHub] drill pull request #775: DRILL-5326: Unit tests failures related to the SERV...

2017-03-07 Thread zfong
Github user zfong commented on a diff in the pull request:

https://github.com/apache/drill/pull/775#discussion_r104805550
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/work/metadata/ServerMetaProvider.java
 ---
@@ -76,7 +76,7 @@
   .setReadOnly(false)
   .setGroupBySupport(GroupBySupport.GB_UNRELATED)
   .setLikeEscapeClauseSupported(true)
-  .setNullCollation(NullCollation.NC_AT_END)
+  .setNullCollation(NullCollation.NC_HIGH)
--- End diff --

@jinfengni  - See @vdiravka's comments in the Jira.  The Drill 
documentation at https://drill.apache.org/docs/order-by-clause/#usage-notes 
says NULLs sort highest.  If the doc is wrong, then we should fix the doc. 


---
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 #748: DRILL-5263: Prevent left NLJoin with non scalar sub...

2017-02-14 Thread zfong
Github user zfong commented on a diff in the pull request:

https://github.com/apache/drill/pull/748#discussion_r101089177
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
 ---
@@ -49,7 +49,7 @@ protected boolean checkPreconditions(DrillJoinRel join, 
RelNode left, RelNode ri
   PlannerSettings settings) {
 JoinRelType type = join.getJoinType();
 
-if (! (type == JoinRelType.INNER || type == JoinRelType.LEFT)) {
+if (!(type == JoinRelType.INNER || (type == JoinRelType.LEFT && 
JoinUtils.hasScalarSubqueryInput(left, right {
--- End diff --

I see.  So, Calcite is converting a query with a subquery into an 
equivalent left join with a scalar subquery.  Do you know if the query returns 
the correct result in that case if a nested loop join is used?  If not, then it 
seems like we should still be returning an error.

Also, how do you distinguish the case where the query has been converted to 
this form by Calcite vs a user explicitly doing a left outer join to a scalar 
subquery.  It seems like in this case, if Drill uses a nested loop join, it 
will also return wrong result.


---
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 #748: DRILL-5263: Prevent left NLJoin with non scalar sub...

2017-02-14 Thread zfong
Github user zfong commented on a diff in the pull request:

https://github.com/apache/drill/pull/748#discussion_r101072686
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
 ---
@@ -49,7 +49,7 @@ protected boolean checkPreconditions(DrillJoinRel join, 
RelNode left, RelNode ri
   PlannerSettings settings) {
 JoinRelType type = join.getJoinType();
 
-if (! (type == JoinRelType.INNER || type == JoinRelType.LEFT)) {
+if (!(type == JoinRelType.INNER || (type == JoinRelType.LEFT && 
JoinUtils.hasScalarSubqueryInput(left, right {
--- End diff --

Don't we want to disallow left outer joins in all cases, even if one of the 
join inputs is a scalar?


---
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 #743: DRILL-5243: Fix TestContextFunctions.sessionIdUDFWithinSam...

2017-02-06 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/743
  
Looks good to me.  +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 #666: DRILL-4956: Temporary tables support

2017-01-20 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/666
  
Checked with @paul-rogers, he's ok with the changes.  I'll add the 
ready-to-commit tag to the Jira.


---
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 #689: DRILL-4938: Need better error message

2016-12-10 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/689
  
Can you add a testcase for this?


---
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 #675: DRILL-5094: Comparator should guarantee transitive attribu...

2016-12-02 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/675
  
Looks good to me.  +1 (non-binding)


---
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 #647: DRILL-4935 Allow Drill to advertise a specific hostname to...

2016-11-09 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/647
  
@harrisonmebane - for pull requests submitted by non-committers, each week, 
one of the committers will do a batch commit of changes that have gone through 
review.  This is normally done towards the end of the week.  There may be an 
exception this week because as @paul-rogers has noted, we're in the middle of 
trying to push out a release candidate build for the 1.9 release.


---
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 #518: DRILL-4653.json - Malformed JSON should not stop the entir...

2016-10-07 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/518
  
@paul-rogers - since you had concerns about particular test cases, and it 
looks like you've confirmed that those are non-issues, would it make sense for 
you to share those with @ssriniva123 and he can then include them as unit tests 
with this pull request?


---
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 #598: DRILL-4906 CASE Expression with constant generates class e...

2016-09-28 Thread zfong
Github user zfong commented on the issue:

https://github.com/apache/drill/pull/598
  
@amansinha100 - can you review this pull request.


---
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: fix obdc => odbc spelling

2016-05-20 Thread zfong
Github user zfong commented on the pull request:

https://github.com/apache/drill/pull/505#issuecomment-220633886
  
@bbevens -- can you review.


---
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: DRILL-4577: Construct a specific path for quer...

2016-04-15 Thread zfong
Github user zfong commented on the pull request:

https://github.com/apache/drill/pull/461#issuecomment-210493669
  
Yes, 8 seconds may still be too long, but it's certainly better than what 
was there before.  We've also been working with Simba to add an "includeSchema" 
option to the ODBC driver so tools that query from INFORMATION_SCHEMA can 
further restrict the query to only the relevant tables within the specified 
schema.  That together with this patch should reduce the time significantly.  
Further improvements may still be needed even beyond this, but I suggest we 
address those incrementally rather than all at once.


---
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: DRILL-4364: Image Metadata Format Plugin

2016-03-24 Thread zfong
Github user zfong commented on the pull request:

https://github.com/apache/drill/pull/367#issuecomment-200871277
  
@vkorukanti, @jaltekruse, @adityakishore - since each of you have written 
format plugins in the past, can one of you help code review this change?


---
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: DRILL-3947: Use setSafe() for date, time, time...

2015-10-19 Thread zfong
Github user zfong commented on the pull request:

https://github.com/apache/drill/pull/208#issuecomment-149259793
  
Aman --  yup, saw that.  Thanks.

On Mon, Oct 19, 2015 at 8:57 AM, Aman Sinha <notificati...@github.com>
wrote:

    > @zfong <https://github.com/zfong>, please see my last explanation about
> why the repro does not occur at small scale. Hence, adding a unit test
> won't help ... unless the static constant that controls the initial size 
of
> the value vectors is made configurable...but that would require more code
> changes.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/drill/pull/208#issuecomment-149257300>.
>



---
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: DRILL-3947: Use setSafe() for date, time, time...

2015-10-18 Thread zfong
Github user zfong commented on the pull request:

https://github.com/apache/drill/pull/208#issuecomment-149087340
  
Is there a small unit test case that reproduces this problem? 


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