[jira] [Commented] (CALCITE-3340) Support Table Function TUMBLE in Planner

2019-10-21 Thread Rui Wang (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956695#comment-16956695
 ] 

Rui Wang commented on CALCITE-3340:
---

That's fair. I will come up with a PR that includes Enumerable implementation 
then.

> Support Table Function TUMBLE in Planner
> 
>
> Key: CALCITE-3340
> URL: https://issues.apache.org/jira/browse/CALCITE-3340
> Project: Calcite
>  Issue Type: Sub-task
>Reporter: Rui Wang
>Assignee: Rui Wang
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 2h 10m
>  Remaining Estimate: 0h
>
> The goal of this JIRA is to generate a logical plan for the following query:
> {code:java}
> SELECT *
> FROM TABLE(TUMBLE(
> TABLE ORDERS,
> INTERVAL '10' MINUTE))
> {code}
> This SQL query does not have DESCRIPTOR included, which is being tracked and 
> discussed by CALCITE-3339.
> I expect we should generate a logical plan from this query that is similar to 
> the following:
> {code:java}
> LogicalProject(ROWTIME=[$0], ID=[$1], PRODUCT=[$2], UNITS=[$3], wstart=[$4], 
> wend=[$5])
>   LogicalTableFunctionScan(invocation=[TUMBLE($3, 6:INTERVAL MINUTE)], 
> rowType=[RecordType(TIMESTAMP(0) ROWTIME, INTEGER ID, VARCHAR(10) PRODUCT, 
> INTEGER UNITS, TIMESTAMP(0) wstart, TIMESTAMP(0) wend)])
> LogicalProject(ROWTIME=[$0], ID=[$1], PRODUCT=[$2], UNITS=[$3])
>   LogicalTableScan(table=[[ORINOCO, ORDERS]])
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3416) SQL Dialects "DEFAULT"s should be more extensible

2019-10-21 Thread Danny Chen (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956650#comment-16956650
 ] 

Danny Chen commented on CALCITE-3416:
-

The constructor has already required an argument of Context, so i have no 
choice to create the context inside the constructor. I choose to make the 
context public static and named "DEFAULT_CONTEXT", the "DEFAULT" instance of 
each SqlDialect reference and create based on the "DEFAULT_CONTEXT".

> SQL Dialects "DEFAULT"s should be more extensible
> -
>
> Key: CALCITE-3416
> URL: https://issues.apache.org/jira/browse/CALCITE-3416
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Steven Talbot
>Assignee: Danny Chen
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> The behavior of SQLDialect is partly governed by the methods defined on the 
> given dialect subclass and partly governed by options passed in in a Context 
> object. So every dialect subclass exposes a "DEFAULT" instance that has been 
> initialized with the correct Context.
> However, if you then wish to extend the dialect for one reason or another, 
> you must create a new instance to extend, or of course create a whole new 
> subclass. In either case, you lose the options from the Context passed into 
> the default, which governs important behavior of the dialect. You can 
> copy-paste the relevant Context out of the Calcite code, but then you lose 
> future improvements or fixes that might land in mainline Calcite.
> It would be nice if each dialect exposed the DEFAULT_CONTEXT that it passed 
> into its DEFAULT instance as a public final member. Then, when extending the 
> dialect, you simply initialize your extension with the DEFAULT_CONTEXT, and 
> if any customization needs to happens on the Context options that's easy to 
> do with the Context's API.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3416) SQL Dialects "DEFAULT"s should be more extensible

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956615#comment-16956615
 ] 

Julian Hyde commented on CALCITE-3416:
--

Yes. The tricky bit is whether those contexts should be static. If they are 
static they can't be referenced easily by a protected method or by a field 
reference. If they are non-static then they have to be created after the 
constructor is called.

Maybe a protected method that is called by the constructor...? Or devise 
something.

> SQL Dialects "DEFAULT"s should be more extensible
> -
>
> Key: CALCITE-3416
> URL: https://issues.apache.org/jira/browse/CALCITE-3416
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Steven Talbot
>Assignee: Danny Chen
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> The behavior of SQLDialect is partly governed by the methods defined on the 
> given dialect subclass and partly governed by options passed in in a Context 
> object. So every dialect subclass exposes a "DEFAULT" instance that has been 
> initialized with the correct Context.
> However, if you then wish to extend the dialect for one reason or another, 
> you must create a new instance to extend, or of course create a whole new 
> subclass. In either case, you lose the options from the Context passed into 
> the default, which governs important behavior of the dialect. You can 
> copy-paste the relevant Context out of the Calcite code, but then you lose 
> future improvements or fixes that might land in mainline Calcite.
> It would be nice if each dialect exposed the DEFAULT_CONTEXT that it passed 
> into its DEFAULT instance as a public final member. Then, when extending the 
> dialect, you simply initialize your extension with the DEFAULT_CONTEXT, and 
> if any customization needs to happens on the Context options that's easy to 
> do with the Context's API.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Comment Edited] (CALCITE-3325) Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl Is Defined As Non Static Causing Memory Leak

2019-10-21 Thread Mehdi Salarkia (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955836#comment-16955836
 ] 

Mehdi Salarkia edited comment on CALCITE-3325 at 10/22/19 2:14 AM:
---

Sorry for the late response as I had been very busy lately.

Here is a code example you can use to see the issue. It would require to 
profile your application (I used Yourkit) or whatever to see the excessive 
number of `org.apache.calcite.avatica.util.UnsynchronizedBuffer` instances.

 
{code:java}
public class AvaticaGC {
private static String URL = "http://localhost:8765;serialization=PROTOBUF;;

public static void main(String[] args) throws Exception {
int threadCount = 40;

ExecutorService executor = Executors.newFixedThreadPool(threadCount);
for (int i = 0; i < threadCount; i++) {

Runnable runner = new Runnable() {
@Override
public void run() {
while (true) {
try (Connection con = 
DriverManager.getConnection("jdbc:phoenix:thin:url=" + URL); PreparedStatement 
psmt = con.prepareStatement("SELECT 1")) {
ResultSet rs = psmt.executeQuery();
rs.next();
Thread.sleep(100);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}
};
executor.execute(runner);
}
executor.shutdown();

}
}
{code}
I saw a GC pause of ~1-2 sec after running this for a few minute (See 
attachments).
  

 
{quote}Finally, let me be super clear: marking the buffer as static is _very 
wrong_. As the name indicates, there is no synchronization of this buffer. If 
you are reusing this buffer, it's going to result in data corruption issues.
{quote}
I'm still having trouble to understand this. What I can see is :
 1) A new instance of UnsynchronizedBuffer is created per connection creation.

2) UnsynchronizedBuffer is stored in thread local (and the previous instance is 
left on `ThreadLocalMap` :( ).

3) The UnsynchronizedBuffer is used during serialize/deserialize.

4) Finally  `UnsynchronizedBuffer.reset()` is invoked to make sure  the 
UnsynchronizedBuffer is ready for next invocation (which never happens because 
on next invocation a new instance will be created :( ). 
{code:java}
private final ThreadLocal threadLocalBuffer =
new ThreadLocal() {
  @Override protected UnsynchronizedBuffer initialValue() {
return new UnsynchronizedBuffer();
  }
};



@Override public byte[] serializeResponse(Response response) throws IOException 
{
  // Avoid BAOS for its synchronized write methods, we don't need that 
concurrency control
  UnsynchronizedBuffer out = threadLocalBuffer.get();
  try {
Message responseMsg = response.serialize();
// Serialization of the response may be large
if (LOG.isTraceEnabled()) {
  LOG.trace("Serializing response '{}'", 
TextFormat.shortDebugString(responseMsg));
}
serializeMessage(out, responseMsg);
return out.toArray();
  } finally {
out.reset();
  }
}

@Override public byte[] serializeRequest(Request request) throws IOException {
  // Avoid BAOS for its synchronized write methods, we don't need that 
concurrency control
  UnsynchronizedBuffer out = threadLocalBuffer.get();
  try {
Message requestMsg = request.serialize();
// Serialization of the request may be large
if (LOG.isTraceEnabled()) {
  LOG.trace("Serializing request '{}'", 
TextFormat.shortDebugString(requestMsg));
}
serializeMessage(out, requestMsg);
return out.toArray();
  } finally {
out.reset();
  }
}{code}
With this code `UnsynchronizedBuffer` is stored at at threadLocalMap and *only* 
the same thread can have access to it, in another word the only chance that an 
instance of `UnsynchronizedBuffer` being re-used is scoped to the same thread 
(no concurrent invocation possible as a thread can serve one request at a 
time). This will run sequentially which is safe as the previous thread have 
already called the *reset* method to clear the buffer.

If you don't want to re-use `UnsynchronizedBuffer` and your logic is have an 
instance of `UnsynchronizedBuffer` per connection why should you even involve 
thread local?

I understand the motive of this design was to prevent thread synchronization 
and lock contention but this only works until you hit a snag with memory and 
see a bad and long GC.

 


was (Author: m2je):
Sorry for the late response as I had been very busy lately.

Here is exactly a code example you can use to see the issue. It would require 
to profile your application (I used Yourkit) or whatever to see the excessive 
number of `org.apache.calcite.avatica.util.UnsynchronizedBuffer` instances.

 

[jira] [Commented] (CALCITE-3325) Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl Is Defined As Non Static Causing Memory Leak

2019-10-21 Thread Mehdi Salarkia (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956610#comment-16956610
 ] 

Mehdi Salarkia commented on CALCITE-3325:
-

{quote}I think this is the source of my confusion. Server-side, we generally 
have a pool of threads which work incoming client requests. This is bounded to 
ensure that the server won't be overrun if there's an influx of clients. If, 
within a single thread, we're not getting the same instance of an 
UnsynchronizedBuffer, then this code doesn't work right.
{quote}
I thought this code is only used by the client but now I can see it is used in 
`AvaticaProtobufHandler` and `AvaticaJsonHandler`. But since only one instance 
of them is created on the server-side you won't have the problem I mentioned 
above in the server code and technically that works exactly the same if you 
define it as static(Referring to 
`org.apache.calcite.avatica.server.AvaticaProtobufHandler#threadLocalBuffer` 
which creates only 1 instance per thread).


{quote}The same _should_ apply client side, but there's more ability for you to 
shoot yourself in the foot (if you run a single operation on a thread and throw 
that thread away). Because you're creating a new Connection every time, you're 
throwing away any state we had for that thread, and re-creating everything. Why 
aren't you keeping that Connection around?
{quote}
We have a connection pool in the actual production code (Hikari) which 
maintains the connections and decide when to kill the connections. To be clear 
this issue only showed itself on the client side with the connection pool size 
of ~200-300 after running the service for multiple hours under heavy load. My 
code harness is an exasperated version of the issue to demonstrate the bug but 
certainly is not the exact code that we use in production.


{quote}I appreciate you sharing the client code. I'll have to try to find some 
time this week to make that functional (I don't have an avatica test harness 
ready to go right now), and can look at that. If you have the cycles, I'd be 
curious what your test shows if you changed it to do..
{quote}
I did run your code snippet and it just creates 40 threadLocal entries (as 
expected) but by the time you don't actually close and open new connections the 
symptoms are not showing as you would expect, because it's not filling up the 
ThreadBuffer and forcing a GC.

For now we have fixed the client side code by overriding the 
`threadLocalBuffer` to be static  and didn't see the issue or any data 
corruptions with a mass load of ~ 4.5 Billion records.  

Thanks again for looking into this and your quick response.

> Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl 
> Is Defined As Non Static Causing Memory Leak
> 
>
> Key: CALCITE-3325
> URL: https://issues.apache.org/jira/browse/CALCITE-3325
> Project: Calcite
>  Issue Type: Bug
>Reporter: Mehdi Salarkia
>Priority: Major
> Attachments: Non-Static.snapshot, Non-static.png, Screen Shot 
> 2019-09-05 at 5.05.20 PM.png, Screen Shot 2019-09-05 at 5.18.19 PM.png, 
> Static.png, Static.snapshot
>
>
> As we were load testing our system on Apache Phoenix via the thin client 
> which uses Avatica we ran into Garbage collection problems. After some 
> investigation we could see there are a lot of unreferenced object due to this 
> variable: 
> {code:java}
> private final ThreadLocal threadLocalBuffer =
> new ThreadLocal() {
>   @Override protected UnsynchronizedBuffer initialValue() {
> return new UnsynchronizedBuffer();
>   }
> };
> {code}
> Which seems to be a reusable buffer for serializing/deserializing the data.
> From my understating there is a copy of this variable created per each 
> instance of ProtobufTranslationImpl. However the proper use of ThreadLocal it 
> seems to be one instance per thread and the current implementation seems to 
> be missing the `static` keyword when defining the thread local variable:
>  See [https://docs.oracle.com/javase/7/docs/api/java/lang/ThreadLocal.html]
> {code:java}
> ThreadLocal instances are typically private static fields in classes that 
> wish to associate state with a thread (e.g., a user ID or Transaction ID).
> {code}
> See the attached snapshot from a memory dump we took.    



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3402) Allow RANGE with compoud ORDER BY clause

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956606#comment-16956606
 ] 

Julian Hyde commented on CALCITE-3402:
--

Only allowing a single key makes a lot of sense.

If {{i}}  is an INTEGER column, you can have {{ORDER BY i RANGE BETWEEN 3 
PRECEDING and 5 FOLLOWING}}. The range is an INTEGER expression. Why? Because 
when you subtract an INTEGER from an INTEGER, the result is an INTEGER.

If {{t}}  is a TIMESTAMP column, you can have {{ORDER BY t RANGE BETWEEN 
INTERVAL '3' DAY PRECEDING and INTERVAL '5' DAY FOLLOWING}}. Why? Because when 
you subtract a TIMESTAMP from a TIMESTAMP, the result is an INTERVAL.

If you have {{x}} and {{y}} are columns of any type, you can't have {{ORDER BY 
x, y RANGE BETWEEN anything AND anything}}, because you can't subtract (x1, y1) 
from (x2, y2).

(By the way, I'm pretty sure that PostgreSQL does allow {{ORDER BY t RANGE 
BETWEEN INTERVAL '3' DAY PRECEDING and INTERVAL '5' DAY FOLLOWING}}, despite 
your assertion that it does not.)

> Allow RANGE with compoud ORDER BY clause
> 
>
> Key: CALCITE-3402
> URL: https://issues.apache.org/jira/browse/CALCITE-3402
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 1.18.0, 1.19.0
>Reporter: benj
>Priority: Major
>
> It will be very useful to have the capacity to use compound ORDER BY clause 
> with RANGE
> {code:sql}
> apache drill (dfs.tmp)> SELECT a
> , last_value(c) OVER(PARTITION BY a ORDER BY c, b DESC RANGE BETWEEN 
> UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)
> FROM (SELECT 1 a, 'b' b, 3 c 
>   UNION SELECT 2, 'c', 4 
>   UNION SELECT 1, 'c', 4
>   /* UNION ... */
>  ) x;
> Error: VALIDATION ERROR: From line 2, column 56 to line 2, column 60: RANGE 
> clause cannot be used with compound ORDER BY clause
> {code}
> I know it's possible (for last_value) to rewrite with first_value  with an 
> reverse ORDER BY and without RANGE to obtain correct result.
> But it will become sometimes less readable and request write from other SGBDR 
> will not be compatible and should be rewrite, and for some other function 
> than last_value, the problem will not be solved like that.
> compound ORDER BY clause with RANGE  is possible with some SGBDR like 
> Postgres: 
> [https://www.postgresql.org/docs/9.3/sql-expressions.html#SYNTAX-WINDOW-FUNCTIONS]



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3416) SQL Dialects "DEFAULT"s should be more extensible

2019-10-21 Thread Danny Chen (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956586#comment-16956586
 ] 

Danny Chen commented on CALCITE-3416:
-

Thanks, i kindly agree, how about we move these contexts into each sql dialect 
as a protected final one ?

> SQL Dialects "DEFAULT"s should be more extensible
> -
>
> Key: CALCITE-3416
> URL: https://issues.apache.org/jira/browse/CALCITE-3416
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Steven Talbot
>Assignee: Danny Chen
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> The behavior of SQLDialect is partly governed by the methods defined on the 
> given dialect subclass and partly governed by options passed in in a Context 
> object. So every dialect subclass exposes a "DEFAULT" instance that has been 
> initialized with the correct Context.
> However, if you then wish to extend the dialect for one reason or another, 
> you must create a new instance to extend, or of course create a whole new 
> subclass. In either case, you lose the options from the Context passed into 
> the default, which governs important behavior of the dialect. You can 
> copy-paste the relevant Context out of the Calcite code, but then you lose 
> future improvements or fixes that might land in mainline Calcite.
> It would be nice if each dialect exposed the DEFAULT_CONTEXT that it passed 
> into its DEFAULT instance as a public final member. Then, when extending the 
> dialect, you simply initialize your extension with the DEFAULT_CONTEXT, and 
> if any customization needs to happens on the Context options that's easy to 
> do with the Context's API.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3340) Support Table Function TUMBLE in Planner

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956587#comment-16956587
 ] 

Julian Hyde commented on CALCITE-3340:
--

Dividing into small steps is fine in one sense, but unfortunately it means that 
you need me to review things several times a week. I get many review requests 
per day, and all of them have to happen outside of business hours. So, I ignore 
most of them.

> Support Table Function TUMBLE in Planner
> 
>
> Key: CALCITE-3340
> URL: https://issues.apache.org/jira/browse/CALCITE-3340
> Project: Calcite
>  Issue Type: Sub-task
>Reporter: Rui Wang
>Assignee: Rui Wang
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 2h 10m
>  Remaining Estimate: 0h
>
> The goal of this JIRA is to generate a logical plan for the following query:
> {code:java}
> SELECT *
> FROM TABLE(TUMBLE(
> TABLE ORDERS,
> INTERVAL '10' MINUTE))
> {code}
> This SQL query does not have DESCRIPTOR included, which is being tracked and 
> discussed by CALCITE-3339.
> I expect we should generate a logical plan from this query that is similar to 
> the following:
> {code:java}
> LogicalProject(ROWTIME=[$0], ID=[$1], PRODUCT=[$2], UNITS=[$3], wstart=[$4], 
> wend=[$5])
>   LogicalTableFunctionScan(invocation=[TUMBLE($3, 6:INTERVAL MINUTE)], 
> rowType=[RecordType(TIMESTAMP(0) ROWTIME, INTEGER ID, VARCHAR(10) PRODUCT, 
> INTEGER UNITS, TIMESTAMP(0) wstart, TIMESTAMP(0) wend)])
> LogicalProject(ROWTIME=[$0], ID=[$1], PRODUCT=[$2], UNITS=[$3])
>   LogicalTableScan(table=[[ORINOCO, ORDERS]])
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3340) Support Table Function TUMBLE in Planner

2019-10-21 Thread Rui Wang (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956484#comment-16956484
 ] 

Rui Wang commented on CALCITE-3340:
---

Friendly ping to raise attention on this JIRA.

> Support Table Function TUMBLE in Planner
> 
>
> Key: CALCITE-3340
> URL: https://issues.apache.org/jira/browse/CALCITE-3340
> Project: Calcite
>  Issue Type: Sub-task
>Reporter: Rui Wang
>Assignee: Rui Wang
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 2h 10m
>  Remaining Estimate: 0h
>
> The goal of this JIRA is to generate a logical plan for the following query:
> {code:java}
> SELECT *
> FROM TABLE(TUMBLE(
> TABLE ORDERS,
> INTERVAL '10' MINUTE))
> {code}
> This SQL query does not have DESCRIPTOR included, which is being tracked and 
> discussed by CALCITE-3339.
> I expect we should generate a logical plan from this query that is similar to 
> the following:
> {code:java}
> LogicalProject(ROWTIME=[$0], ID=[$1], PRODUCT=[$2], UNITS=[$3], wstart=[$4], 
> wend=[$5])
>   LogicalTableFunctionScan(invocation=[TUMBLE($3, 6:INTERVAL MINUTE)], 
> rowType=[RecordType(TIMESTAMP(0) ROWTIME, INTEGER ID, VARCHAR(10) PRODUCT, 
> INTEGER UNITS, TIMESTAMP(0) wstart, TIMESTAMP(0) wend)])
> LogicalProject(ROWTIME=[$0], ID=[$1], PRODUCT=[$2], UNITS=[$3])
>   LogicalTableScan(table=[[ORINOCO, ORDERS]])
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3428) Refine RelMdColumnUniqueness for Filter by considering constant columns

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956355#comment-16956355
 ] 

Julian Hyde commented on CALCITE-3428:
--

Can you make it more efficient when there are no constants or no pulled-up 
predicates? A lot of bit sets and lists are rebuilt for no reason.

{{deduceConstantColumnsFromPredicates}} needs an explanation.

> Refine RelMdColumnUniqueness for Filter by considering constant columns
> ---
>
> Key: CALCITE-3428
> URL: https://issues.apache.org/jira/browse/CALCITE-3428
> Project: Calcite
>  Issue Type: Improvement
>Reporter: jin xing
>Assignee: jin xing
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> AggregateRemoveRule fails to remove the top Aggregate for below SQL
> {code:java}
> select mgr, sum(sum_sal)
> from
>  (select mgr, deptno, sum(sal) sum_sal
>  from sales.emp
>  group by mgr, deptno)
> where deptno=100
> group by mgr
> {code}
> The reason is that RelMdColumnUniqueness doesn't take the filtering condition 
> into consideration when checking uniqueness of columns. 
> This PR proposes to refine RelMdColumnUniqueness for Filter, thus to 
> strengthen AggregateRemoveRule.
> Resolving this Jira will help a lot for CALCITE-3334 by removing the 
> redundant compensation Aggregate when doing materialization matching



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3435) floor(mod(33.5,7)) wrongly returns 5.5

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956325#comment-16956325
 ] 

Julian Hyde commented on CALCITE-3435:
--

What do you think should be the return type of FLOOR applied to an argument of 
DECIMAL(4, 1)?

> floor(mod(33.5,7)) wrongly returns 5.5
> --
>
> Key: CALCITE-3435
> URL: https://issues.apache.org/jira/browse/CALCITE-3435
> Project: Calcite
>  Issue Type: Bug
>Affects Versions: 1.16.0, 1.21.0
>Reporter: jiezouSH
>Priority: Minor
>
> mod's return type is
> chain(DECIMAL_MOD_NULLABLE, ARG1_NULLABLE),
> but mod(33.5,7)'s result is 5.5, not in line with ARG1_NULLABLE.
> This causes floor(mod(33.5,7)) wrongly returns 5.5



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3416) SQL Dialects "DEFAULT"s should be more extensible

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956339#comment-16956339
 ] 

Julian Hyde commented on CALCITE-3416:
--

Thanks for creating https://github.com/apache/calcite/pull/1524, [~danny0405]. 
But I don't like the approach, because it centralizes the code for all 
dialects. The initialization code for each dialect should be within its dialect 
class. Also, people should be able to create their own dialect sub-class 
without modifying a Calcite-controlled enum.

> SQL Dialects "DEFAULT"s should be more extensible
> -
>
> Key: CALCITE-3416
> URL: https://issues.apache.org/jira/browse/CALCITE-3416
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Steven Talbot
>Assignee: Danny Chen
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 10m
>  Remaining Estimate: 0h
>
> The behavior of SQLDialect is partly governed by the methods defined on the 
> given dialect subclass and partly governed by options passed in in a Context 
> object. So every dialect subclass exposes a "DEFAULT" instance that has been 
> initialized with the correct Context.
> However, if you then wish to extend the dialect for one reason or another, 
> you must create a new instance to extend, or of course create a whole new 
> subclass. In either case, you lose the options from the Context passed into 
> the default, which governs important behavior of the dialect. You can 
> copy-paste the relevant Context out of the Calcite code, but then you lose 
> future improvements or fixes that might land in mainline Calcite.
> It would be nice if each dialect exposed the DEFAULT_CONTEXT that it passed 
> into its DEFAULT instance as a public final member. Then, when extending the 
> dialect, you simply initialize your extension with the DEFAULT_CONTEXT, and 
> if any customization needs to happens on the Context options that's easy to 
> do with the Context's API.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3395) add BuiltinMethod for Substring(String, int)

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3395?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956333#comment-16956333
 ] 

Julian Hyde commented on CALCITE-3395:
--

I agree, it would be useful if we had an entry in BuiltInMethod for each 
overload.

> add BuiltinMethod for Substring(String, int)
> 
>
> Key: CALCITE-3395
> URL: https://issues.apache.org/jira/browse/CALCITE-3395
> Project: Calcite
>  Issue Type: Improvement
>  Components: core
>Reporter: Youjun Yuan
>Priority: Minor
>   Original Estimate: 4h
>  Remaining Estimate: 4h
>
> substring function has two versoins:
> 1, Substring(String, int, int)
> 2, Substring(String, int)
> currently in BuiltinMethod.java, only the first one is defined. Need to 
> define the second one as well, so that we can use both of them.
> Apache Flink(FunctionGenerator.scala), reference the BuiltinMethod, since 
> Calcite only defines 1, not 2, Flink always resolve SUBSTRING to 
> Substring(String, int, int) even if there is only 2 parameters. This problem 
> happens to be covered by method overloading of java, but it's still a 
> potential problem.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956327#comment-16956327
 ] 

Julian Hyde commented on CALCITE-3433:
--

The query's answer is wrong. Yes, this is a bug. I was speculating as to the 
cause of the bug.

> EQUALS operator between date/timestamp types returns false if the type is 
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Priority: Major
>
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as 
> time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as 
> timestamp) as time0,cast(null as timestamp) as time1) calcs
> answer is false
> but 
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as 
> time0,timestamp'2000-12-30 21:07:32'as time1) calcs
> answer is true
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3293) Add strcmp function

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956326#comment-16956326
 ] 

Julian Hyde commented on CALCITE-3293:
--

Yes, that's Oracle Golden Gate, not Oracle SQL.

> Add strcmp function
> ---
>
> Key: CALCITE-3293
> URL: https://issues.apache.org/jira/browse/CALCITE-3293
> Project: Calcite
>  Issue Type: Improvement
>Reporter: xzh_dz
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> Add strcmp function.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3428) Refine RelMdColumnUniqueness for Filter by considering constant columns

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956318#comment-16956318
 ] 

Julian Hyde commented on CALCITE-3428:
--

I see there is just one commit in the PR. When revising PRs after review please 
add new commits. Otherwise we have to review everything again.

> Refine RelMdColumnUniqueness for Filter by considering constant columns
> ---
>
> Key: CALCITE-3428
> URL: https://issues.apache.org/jira/browse/CALCITE-3428
> Project: Calcite
>  Issue Type: Improvement
>Reporter: jin xing
>Assignee: jin xing
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> AggregateRemoveRule fails to remove the top Aggregate for below SQL
> {code:java}
> select mgr, sum(sum_sal)
> from
>  (select mgr, deptno, sum(sal) sum_sal
>  from sales.emp
>  group by mgr, deptno)
> where deptno=100
> group by mgr
> {code}
> The reason is that RelMdColumnUniqueness doesn't take the filtering condition 
> into consideration when checking uniqueness of columns. 
> This PR proposes to refine RelMdColumnUniqueness for Filter, thus to 
> strengthen AggregateRemoveRule.
> Resolving this Jira will help a lot for CALCITE-3334 by removing the 
> redundant compensation Aggregate when doing materialization matching



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (CALCITE-3436) CalciteConnectionConfigImpl.set obliterates previous property values

2019-10-21 Thread Ryan Fu (Jira)
Ryan Fu created CALCITE-3436:


 Summary: CalciteConnectionConfigImpl.set obliterates previous 
property values
 Key: CALCITE-3436
 URL: https://issues.apache.org/jira/browse/CALCITE-3436
 Project: Calcite
  Issue Type: Bug
Reporter: Ryan Fu


The cause is that `new Properties(properties)` doesn't copy the previous value 
as, say, `new HashMap(map)` does.

 
 * Add a unit test in PlannerTest, that if you call set(x, 1) then set(y, 2) on 
a CalciteConnectionConfigImpl, the result contains x and y. 
 * Consider potentially using `clone()` as Properties implements Cloneable



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3325) Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl Is Defined As Non Static Causing Memory Leak

2019-10-21 Thread Josh Elser (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16956310#comment-16956310
 ] 

Josh Elser commented on CALCITE-3325:
-

{quote}Finally  `UnsynchronizedBuffer.reset()` is invoked to make sure  the 
UnsynchronizedBuffer is ready for next invocation (which never happens because 
on next invocation a new instance will be created :( ). 
{quote}
I think this is the source of my confusion. Server-side, we generally have a 
pool of threads which work incoming client requests. This is bounded to ensure 
that the server won't be overrun if there's an influx of clients. If, within a 
single thread, we're not getting the same instance of an UnsynchronizedBuffer, 
then this code doesn't work right.

The same _should_ apply client side, but there's more ability for you to shoot 
yourself in the foot (if you run a single operation on a thread and throw that 
thread away). Because you're creating a new Connection every time, you're 
throwing away any state we had for that thread, and re-creating everything. Why 
aren't you keeping that Connection around?

The ThreadLocal is intended to be used as a "poor-man's" pool. I intended the 
ThreadLocal to be a way that scales linearly with what the client is doing – 
not that you, as the developer, have to be aware of some arcane configuration 
property that you have to scale up as you vertically scale up your Avatica 
client code (if that makes sense).

I appreciate you sharing the client code. I'll have to try to find some time 
this week to make that functional (I don't have an avatica test harness ready 
to go right now), and can look at that. If you have the cycles, I'd be curious 
what your test shows if you changed it to do..
{code:java}
public void run() {
  try (Connection con = 
DriverManager.getConnection("jdbc:phoenix:thin:url=" + URL) {
while (true) {
  try (PreparedStatement psmt = 
con.prepareStatement("SELECT 1")) {
ResultSet rs = psmt.executeQuery();
rs.next();
Thread.sleep(100);
  }
}
  } catch (Exception e) { throw new RuntimeException(e); }
}
{code}

> Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl 
> Is Defined As Non Static Causing Memory Leak
> 
>
> Key: CALCITE-3325
> URL: https://issues.apache.org/jira/browse/CALCITE-3325
> Project: Calcite
>  Issue Type: Bug
>Reporter: Mehdi Salarkia
>Priority: Major
> Attachments: Non-Static.snapshot, Non-static.png, Screen Shot 
> 2019-09-05 at 5.05.20 PM.png, Screen Shot 2019-09-05 at 5.18.19 PM.png, 
> Static.png, Static.snapshot
>
>
> As we were load testing our system on Apache Phoenix via the thin client 
> which uses Avatica we ran into Garbage collection problems. After some 
> investigation we could see there are a lot of unreferenced object due to this 
> variable: 
> {code:java}
> private final ThreadLocal threadLocalBuffer =
> new ThreadLocal() {
>   @Override protected UnsynchronizedBuffer initialValue() {
> return new UnsynchronizedBuffer();
>   }
> };
> {code}
> Which seems to be a reusable buffer for serializing/deserializing the data.
> From my understating there is a copy of this variable created per each 
> instance of ProtobufTranslationImpl. However the proper use of ThreadLocal it 
> seems to be one instance per thread and the current implementation seems to 
> be missing the `static` keyword when defining the thread local variable:
>  See [https://docs.oracle.com/javase/7/docs/api/java/lang/ThreadLocal.html]
> {code:java}
> ThreadLocal instances are typically private static fields in classes that 
> wish to associate state with a thread (e.g., a user ID or Transaction ID).
> {code}
> See the attached snapshot from a memory dump we took.    



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Comment Edited] (CALCITE-3428) Refine RelMdColumnUniqueness for Filter by considering constant columns

2019-10-21 Thread jin xing (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955719#comment-16955719
 ] 

jin xing edited comment on CALCITE-3428 at 10/21/19 2:23 PM:
-

Thanks a lot [~julianhyde] ~
 I updated the PR by your comments ~

Please take another look when you have time ~


was (Author: jinxing6...@126.com):
Thanks a lot [~julianhyde] ~
I will refine the PR by your comment !

> Refine RelMdColumnUniqueness for Filter by considering constant columns
> ---
>
> Key: CALCITE-3428
> URL: https://issues.apache.org/jira/browse/CALCITE-3428
> Project: Calcite
>  Issue Type: Improvement
>Reporter: jin xing
>Assignee: jin xing
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 40m
>  Remaining Estimate: 0h
>
> AggregateRemoveRule fails to remove the top Aggregate for below SQL
> {code:java}
> select mgr, sum(sum_sal)
> from
>  (select mgr, deptno, sum(sal) sum_sal
>  from sales.emp
>  group by mgr, deptno)
> where deptno=100
> group by mgr
> {code}
> The reason is that RelMdColumnUniqueness doesn't take the filtering condition 
> into consideration when checking uniqueness of columns. 
> This PR proposes to refine RelMdColumnUniqueness for Filter, thus to 
> strengthen AggregateRemoveRule.
> Resolving this Jira will help a lot for CALCITE-3334 by removing the 
> redundant compensation Aggregate when doing materialization matching



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3416) SQL Dialects "DEFAULT"s should be more extensible

2019-10-21 Thread ASF GitHub Bot (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

ASF GitHub Bot updated CALCITE-3416:

Labels: pull-request-available  (was: )

> SQL Dialects "DEFAULT"s should be more extensible
> -
>
> Key: CALCITE-3416
> URL: https://issues.apache.org/jira/browse/CALCITE-3416
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Steven Talbot
>Assignee: Danny Chen
>Priority: Minor
>  Labels: pull-request-available
>
> The behavior of SQLDialect is partly governed by the methods defined on the 
> given dialect subclass and partly governed by options passed in in a Context 
> object. So every dialect subclass exposes a "DEFAULT" instance that has been 
> initialized with the correct Context.
> However, if you then wish to extend the dialect for one reason or another, 
> you must create a new instance to extend, or of course create a whole new 
> subclass. In either case, you lose the options from the Context passed into 
> the default, which governs important behavior of the dialect. You can 
> copy-paste the relevant Context out of the Calcite code, but then you lose 
> future improvements or fixes that might land in mainline Calcite.
> It would be nice if each dialect exposed the DEFAULT_CONTEXT that it passed 
> into its DEFAULT instance as a public final member. Then, when extending the 
> dialect, you simply initialize your extension with the DEFAULT_CONTEXT, and 
> if any customization needs to happens on the Context options that's easy to 
> do with the Context's API.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3325) Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl Is Defined As Non Static Causing Memory Leak

2019-10-21 Thread Mehdi Salarkia (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3325?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955836#comment-16955836
 ] 

Mehdi Salarkia commented on CALCITE-3325:
-

Sorry for the late response as I had been very busy lately.

Here is exactly a code example you can use to see the issue. It would require 
to profile your application (I used Yourkit) or whatever to see the excessive 
number of `org.apache.calcite.avatica.util.UnsynchronizedBuffer` instances.

 
{code:java}
public class AvaticaGC {
private static String URL = "http://localhost:8765;serialization=PROTOBUF;;

public static void main(String[] args) throws Exception {
int threadCount = 40;

ExecutorService executor = Executors.newFixedThreadPool(threadCount);
for (int i = 0; i < threadCount; i++) {

Runnable runner = new Runnable() {
@Override
public void run() {
while (true) {
try (Connection con = 
DriverManager.getConnection("jdbc:phoenix:thin:url=" + URL); PreparedStatement 
psmt = con.prepareStatement("SELECT 1")) {
ResultSet rs = psmt.executeQuery();
rs.next();
Thread.sleep(100);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}
};
executor.execute(runner);
}
executor.shutdown();

}
}
{code}
I saw a GC pause of ~1-2 sec after running this for a few minute (See 
attachments).
 

 
{quote}Finally, let me be super clear: marking the buffer as static is _very 
wrong_. As the name indicates, there is no synchronization of this buffer. If 
you are reusing this buffer, it's going to result in data corruption issues.
{quote}
I'm still having trouble to understand this. What I can see is :
1) A new instance of UnsynchronizedBuffer is created per connection creation.

2) UnsynchronizedBuffer is stored in thread local (and the previous instance is 
left on `ThreadLocalMap` :( ).

3) The UnsynchronizedBuffer is used during serialize/deserialize.

4) Finally  `UnsynchronizedBuffer.reset()` is invoked to make sure  the 
UnsynchronizedBuffer is ready for next invocation (which never happens because 
on next invocation a new instance will be created :( ). 
{code:java}
private final ThreadLocal threadLocalBuffer =
new ThreadLocal() {
  @Override protected UnsynchronizedBuffer initialValue() {
return new UnsynchronizedBuffer();
  }
};



@Override public byte[] serializeResponse(Response response) throws IOException 
{
  // Avoid BAOS for its synchronized write methods, we don't need that 
concurrency control
  UnsynchronizedBuffer out = threadLocalBuffer.get();
  try {
Message responseMsg = response.serialize();
// Serialization of the response may be large
if (LOG.isTraceEnabled()) {
  LOG.trace("Serializing response '{}'", 
TextFormat.shortDebugString(responseMsg));
}
serializeMessage(out, responseMsg);
return out.toArray();
  } finally {
out.reset();
  }
}

@Override public byte[] serializeRequest(Request request) throws IOException {
  // Avoid BAOS for its synchronized write methods, we don't need that 
concurrency control
  UnsynchronizedBuffer out = threadLocalBuffer.get();
  try {
Message requestMsg = request.serialize();
// Serialization of the request may be large
if (LOG.isTraceEnabled()) {
  LOG.trace("Serializing request '{}'", 
TextFormat.shortDebugString(requestMsg));
}
serializeMessage(out, requestMsg);
return out.toArray();
  } finally {
out.reset();
  }
}{code}
With this code `UnsynchronizedBuffer` is stored at at threadLocalMap and *only* 
the same thread can have access to it, in another word the only chance that an 
instance of `UnsynchronizedBuffer` being re-used is scoped to the same thread 
(no concurrent invocation possible as a thread can serve one request at a 
time). This will run sequentially which is safe as the previous thread have 
already called the *reset* method to clear the buffer.

If you don't want to re-use `UnsynchronizedBuffer` and your logic is have an 
instance of `UnsynchronizedBuffer` per connection why should you even involve 
thread local?

I understand the motive of [this 
design|https://issues.apache.org/jira/browse/CALCITE-1094 ] was to prevent 
thread synchronization and lock contention but this only works until you hit a 
snag with memory and see a bad and long GC.

 

> Thread Local Buffer Variable (threadLocalBuffer) In ProtobufTranslationImpl 
> Is Defined As Non Static Causing Memory Leak
> 
>
> Key: CALCITE-3325
> URL: 

[jira] [Assigned] (CALCITE-3416) SQL Dialects "DEFAULT"s should be more extensible

2019-10-21 Thread Danny Chen (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3416?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Danny Chen reassigned CALCITE-3416:
---

Assignee: Danny Chen

> SQL Dialects "DEFAULT"s should be more extensible
> -
>
> Key: CALCITE-3416
> URL: https://issues.apache.org/jira/browse/CALCITE-3416
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Steven Talbot
>Assignee: Danny Chen
>Priority: Minor
>
> The behavior of SQLDialect is partly governed by the methods defined on the 
> given dialect subclass and partly governed by options passed in in a Context 
> object. So every dialect subclass exposes a "DEFAULT" instance that has been 
> initialized with the correct Context.
> However, if you then wish to extend the dialect for one reason or another, 
> you must create a new instance to extend, or of course create a whole new 
> subclass. In either case, you lose the options from the Context passed into 
> the default, which governs important behavior of the dialect. You can 
> copy-paste the relevant Context out of the Calcite code, but then you lose 
> future improvements or fixes that might land in mainline Calcite.
> It would be nice if each dialect exposed the DEFAULT_CONTEXT that it passed 
> into its DEFAULT instance as a public final member. Then, when extending the 
> dialect, you simply initialize your extension with the DEFAULT_CONTEXT, and 
> if any customization needs to happens on the Context options that's easy to 
> do with the Context's API.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (CALCITE-3435) floor(mod(33.5,7)) wrongly returns 5.5

2019-10-21 Thread jiezouSH (Jira)
jiezouSH created CALCITE-3435:
-

 Summary: floor(mod(33.5,7)) wrongly returns 5.5
 Key: CALCITE-3435
 URL: https://issues.apache.org/jira/browse/CALCITE-3435
 Project: Calcite
  Issue Type: Bug
Affects Versions: 1.21.0, 1.16.0
Reporter: jiezouSH


mod's return type is

chain(DECIMAL_MOD_NULLABLE, ARG1_NULLABLE),

but mod(33.5,7)'s result is 5.5, not in line with ARG1_NULLABLE.

This causes floor(mod(33.5,7)) wrongly returns 5.5



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3429) Support MAP type for JavaToSqlTypeConversionRules

2019-10-21 Thread Wang Yanlin (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3429?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Wang Yanlin updated CALCITE-3429:
-
Summary: Support MAP type for JavaToSqlTypeConversionRules  (was: 
Refinement for implementation of sql type factory)

> Support MAP type for JavaToSqlTypeConversionRules
> -
>
> Key: CALCITE-3429
> URL: https://issues.apache.org/jira/browse/CALCITE-3429
> Project: Calcite
>  Issue Type: Improvement
>Reporter: Wang Yanlin
>Assignee: Wang Yanlin
>Priority: Major
>  Labels: pull-request-available
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> 1. Add map info from Java Map class to SqlTypeName.Map
> 2. SqlTypeName.Map is not a basic type, should not be used in factory method 
> *createSqlType*, just like SqlTypeName.Array



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3293) Add strcmp function

2019-10-21 Thread Danny Chen (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955817#comment-16955817
 ] 

Danny Chen commented on CALCITE-3293:
-

Sorry, my fault, the Oracle one seems with "@" as prefix.

> Add strcmp function
> ---
>
> Key: CALCITE-3293
> URL: https://issues.apache.org/jira/browse/CALCITE-3293
> Project: Calcite
>  Issue Type: Improvement
>Reporter: xzh_dz
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> Add strcmp function.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3293) Add strcmp function

2019-10-21 Thread Danny Chen (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955816#comment-16955816
 ] 

Danny Chen commented on CALCITE-3293:
-

I found one in the Oracle doc: 
https://docs.oracle.com/goldengate/1212/gg-winux/GWURF/column_conversion_functions024.htm#GWURF814

> Add strcmp function
> ---
>
> Key: CALCITE-3293
> URL: https://issues.apache.org/jira/browse/CALCITE-3293
> Project: Calcite
>  Issue Type: Improvement
>Reporter: xzh_dz
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> Add strcmp function.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable

2019-10-21 Thread jiezouSH (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955813#comment-16955813
 ] 

jiezouSH commented on CALCITE-3433:
---

[~julianhyde]  Yes, I have found that TIMESTAMP is being stored as a Long, and 
Date is being stored as a Integer and  == is used to compare Long/Integer. But 
the query's answer is really confusing.

> EQUALS operator between date/timestamp types returns false if the type is 
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Priority: Major
>
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as 
> time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as 
> timestamp) as time0,cast(null as timestamp) as time1) calcs
> answer is false
> but 
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as 
> time0,timestamp'2000-12-30 21:07:32'as time1) calcs
> answer is true
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3293) Add strcmp function

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955810#comment-16955810
 ] 

Julian Hyde commented on CALCITE-3293:
--

Does Oracle SQL have this function? I don’t think so. 

> Add strcmp function
> ---
>
> Key: CALCITE-3293
> URL: https://issues.apache.org/jira/browse/CALCITE-3293
> Project: Calcite
>  Issue Type: Improvement
>Reporter: xzh_dz
>Priority: Minor
>  Labels: pull-request-available
>  Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> Add strcmp function.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Commented] (CALCITE-3433) EQUALS operator between date/timestamp types returns false if the type is nullable

2019-10-21 Thread Julian Hyde (Jira)


[ 
https://issues.apache.org/jira/browse/CALCITE-3433?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16955806#comment-16955806
 ] 

Julian Hyde commented on CALCITE-3433:
--

I wonder if the TIMESTAMP value is being stored as a Long (rather than long) 
because it is nullable, and we are using == to compare rather than 
Object.equals. 

> EQUALS operator between date/timestamp types returns false if the type is 
> nullable
> --
>
> Key: CALCITE-3433
> URL: https://issues.apache.org/jira/browse/CALCITE-3433
> Project: Calcite
>  Issue Type: Bug
>  Components: core
>Affects Versions: 1.17.0, 1.18.0, 1.19.0, 1.20.0, 1.21.0
>Reporter: jiezouSH
>Priority: Major
>
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as 
> time0,timestamp'2000-12-30 21:07:32'as time1 union all select cast(null as 
> timestamp) as time0,cast(null as timestamp) as time1) calcs
> answer is false
> but 
> sql
> select time0=time1 from (select timestamp'2000-12-30 21:07:32'as 
> time0,timestamp'2000-12-30 21:07:32'as time1) calcs
> answer is true
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3434) elasticsearch adapter can not init rest client with "pathPrefix"

2019-10-21 Thread Jeffery Zhang (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jeffery Zhang updated CALCITE-3434:
---
Description: 
when we use elasticsearch adapter, we can only set coordinate or hosts, but we 
can't set "pathPrefix" to the rest client.
  can we set the calcite model like below,
 {
 "version": "1.0",
 "defaultSchema": "elasticsearch",
 "schemas": [
   {
     "type": "custom",
     "name": "elasticsearch",
     "factory": 
"org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
     "operand": {                

        "coordinates": "\{'127.0.0.1': 9200}",
         "index":"usa",
         "pathPrefix":"pathPrefix"     

    } 

 }]

}

  was:
when we use elasticsearch adapter, we can only set coordinate or hosts, but we 
can't set "pathPrefix" to the rest client.
  can we set the calcite model like below,
 {
 "version": "1.0",
 "defaultSchema": "elasticsearch",
 "schemas": [
   {
     "type": "custom",
     "name": "elasticsearch",
     "factory": 
"org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
     "operand": {        

        "coordinates": "\{'127.0.0.1': 9200}",
         "index":"usa",
         "pathPrefix":"pathPrefix"     

    } 

 }]

}


> elasticsearch  adapter can not init rest client with "pathPrefix"
> -
>
> Key: CALCITE-3434
> URL: https://issues.apache.org/jira/browse/CALCITE-3434
> Project: Calcite
>  Issue Type: Improvement
>  Components: elasticsearch-adapter
>Affects Versions: 1.21.0
>Reporter: Jeffery Zhang
>Priority: Minor
>  Labels: pull-request-available
> Fix For: next
>
>
> when we use elasticsearch adapter, we can only set coordinate or hosts, but 
> we can't set "pathPrefix" to the rest client.
>   can we set the calcite model like below,
>  {
>  "version": "1.0",
>  "defaultSchema": "elasticsearch",
>  "schemas": [
>    {
>      "type": "custom",
>      "name": "elasticsearch",
>      "factory": 
> "org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
>      "operand": {                
>         "coordinates": "\{'127.0.0.1': 9200}",
>          "index":"usa",
>          "pathPrefix":"pathPrefix"     
>     } 
>  }]
> }



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Updated] (CALCITE-3434) elasticsearch adapter can not init rest client with "pathPrefix"

2019-10-21 Thread Jeffery Zhang (Jira)


 [ 
https://issues.apache.org/jira/browse/CALCITE-3434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jeffery Zhang updated CALCITE-3434:
---
Description: 
when we use elasticsearch adapter, we can only set coordinate or hosts, but we 
can't set "pathPrefix" to the rest client.
  can we set the calcite model like below,
 {
 "version": "1.0",
 "defaultSchema": "elasticsearch",
 "schemas": [
   {
     "type": "custom",
     "name": "elasticsearch",
     "factory": 
"org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
     "operand": {        

        "coordinates": "\{'127.0.0.1': 9200}",
         "index":"usa",
         "pathPrefix":"pathPrefix"     

    } 

 }]

}

  was:
when we use elasticsearch adapter, we can only set coordinate or hosts, but we 
can't set "pathPrefix" to the rest client.
 can we set the calcite model like below,
{
"version": "1.0",
"defaultSchema": "elasticsearch",
"schemas": [
  {
    "type": "custom",
    "name": "elasticsearch",
    "factory": 
"org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
    "operand": {
        "coordinates": "\{'127.0.0.1': 9200}",
        "index":"usa",
        "pathPrefix":"pathPrefix"     }   }]}


> elasticsearch  adapter can not init rest client with "pathPrefix"
> -
>
> Key: CALCITE-3434
> URL: https://issues.apache.org/jira/browse/CALCITE-3434
> Project: Calcite
>  Issue Type: Improvement
>  Components: elasticsearch-adapter
>Affects Versions: 1.21.0
>Reporter: Jeffery Zhang
>Priority: Minor
>  Labels: pull-request-available
> Fix For: next
>
>
> when we use elasticsearch adapter, we can only set coordinate or hosts, but 
> we can't set "pathPrefix" to the rest client.
>   can we set the calcite model like below,
>  {
>  "version": "1.0",
>  "defaultSchema": "elasticsearch",
>  "schemas": [
>    {
>      "type": "custom",
>      "name": "elasticsearch",
>      "factory": 
> "org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
>      "operand": {        
>         "coordinates": "\{'127.0.0.1': 9200}",
>          "index":"usa",
>          "pathPrefix":"pathPrefix"     
>     } 
>  }]
> }



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


[jira] [Created] (CALCITE-3434) elasticsearch adapter can not init rest client with "pathPrefix"

2019-10-21 Thread Jeffery Zhang (Jira)
Jeffery Zhang created CALCITE-3434:
--

 Summary: elasticsearch  adapter can not init rest client with 
"pathPrefix"
 Key: CALCITE-3434
 URL: https://issues.apache.org/jira/browse/CALCITE-3434
 Project: Calcite
  Issue Type: Improvement
  Components: elasticsearch-adapter
Affects Versions: 1.21.0
Reporter: Jeffery Zhang
 Fix For: next


when we use elasticsearch adapter, we can only set coordinate or hosts, but we 
can't set "pathPrefix" to the rest client.
 can we set the calcite model like below,
{
"version": "1.0",
"defaultSchema": "elasticsearch",
"schemas": [
  {
    "type": "custom",
    "name": "elasticsearch",
    "factory": 
"org.apache.calcite.adapter.elasticsearch.ElasticsearchSchemaFactory",
    "operand": {
        "coordinates": "\{'127.0.0.1': 9200}",
        "index":"usa",
        "pathPrefix":"pathPrefix"     }   }]}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)