Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Vladimir Sitnikov]

Michael>wee could always pull the relevan code into Calcite

Do you realize Checkstyle does not support source-code based plugins?
It requires jar files. It would be non-trivial to build a custom checkstyle
plugin in the beginning of Calcite build and use it later in the same build.

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Vladimir Sitnikov]

Danny> it normalize the file end pretty well(avoid all kinds of file end)

I just realized there's JIRA which answers it:
https://issues.apache.org/jira/browse/CALCITE-490

One can't avoid 'file end'. The file has to end with something, and the use
of '// End' does not avoid file end.

'// End file.ext' is especially painful when refactoring code in IDE (e.g.
file renames)

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Vladimir Sitnikov]

Project-specific rules are missing, thus general rules apply:
https://www.apache.org/foundation/voting.html#votes-on-code-modification ,
https://www.apache.org/foundation/voting.html

It says:

> In this scenario, a negative vote constitutes a veto
 , which cannot be
overridden

>Under normal (non-lazy consensus) conditions, the proposal requires three
positive votes and no negative ones in order to pass

I should have declared the vote as lazy consensus, but I realized that only
after it was too late.

So the effective rules for the current vote are: three +1 to pass, no -1,
-1 must be accompanied with a technical justification.

My vote is +1, so two more +1 are required.

Julian> Assuming that this was a discussion, my “-1” meant “I’m generally
against this idea”

It is a vote on a code change, thus '-1' implies veto.

Julian> without more discussion

What do you mean by that? Is the current 'discussion' enough?

Vladimir

[jira] [Created] (CALCITE-3570) Support MutableSnapshot in MutableRel

[xzh_dz (Jira)]

xzh_dz created CALCITE-3570:
---

 Summary: Support MutableSnapshot in MutableRel
 Key: CALCITE-3570
 URL: https://issues.apache.org/jira/browse/CALCITE-3570
 Project: Calcite
  Issue Type: Wish
Reporter: xzh_dz


Support MutableSnapshot in MutableRel.This PR implements mutual conversion 
between mutablesnapshot and logicalsnapshot.



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

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Danny Chan]

I’m not voting, I just want to say something to this HydromaticFileSetCheck

>* HydromaticFileSetCheck produces hard to understand messages. I guess many
of you have faced "Open parentheses exceed closes by 2 or more
[HydromaticFileSet]" issue, and I guess you it took you a lot to understand
what the error really means.
* HydromaticFileSetCheck duplicates logic which is already implemented
elsewhere (e.g. fail on tab characters, end with newline)
* HydromaticFileSetCheck produces errors when automatic correction is
possible. For instance, "@Override should not be on its own line"
* "// End ..." file footer is redundant, and sometimes it is misleading
(e.g. // End Parser.jj is retained even in the generated parser which makes
no sense).


1. “Open parentheses exceed closes by 2 or more” seems not very hard to 
understand, actually straightforward for me.
2.  "// End ..." file footer is redundant, and sometimes it is misleading: I 
don’t think it is redundant, it normalize the file end pretty well(avoid all 
kinds of file end), personally I like this style.
3. “ // End Parser.jj is retained even in the generated parser which makes
no sense” I think this is not that much important.


Best,
Danny Chan
在 2019年12月4日 +0800 PM6:31，dev@calcite.apache.org，写道：
>
> * HydromaticFileSetCheck produces hard to understand messages. I guess many
> of you have faced "Open parentheses exceed closes by 2 or more
> [HydromaticFileSet]" issue, and I guess you it took you a lot to understand
> what the error really means.
> * HydromaticFileSetCheck duplicates logic which is already implemented
> elsewhere (e.g. fail on tab characters, end with newline)
> * HydromaticFileSetCheck produces errors when automatic correction is
> possible. For instance, "@Override should not be on its own line"
> * "// End ..." file footer is redundant, and sometimes it is misleading
> (e.g. // End Parser.jj is retained even in the generated parser which makes
> no sense).

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Julian Hyde]

Vladimir created the confusion by starting a vote on a commit, and without 
specifying the rules of the vote (unanimous, majority, lazy consensus). Then 
later on he referred to it as a discussion.

Assuming that this was a discussion, my “-1” meant “I’m generally against this 
idea”.

It’s consistent with what I wrote in the PR [1], "Please don't commit this 
without more discussion.”

Julian

[1] https://github.com/apache/calcite/pull/1625 


> On Dec 4, 2019, at 5:10 PM, Michael Mior  wrote:
> 
> I'll leave Julian to clarify whether or not the -1 was intended to be
> a veto, but as you noted, Julian expressed some concern in the PR and
> the associated JIRA. If we are voting, I'm currently -0 because I
> haven't found checkstyle violations to be significant problem for me
> and I don't like the idea of losing checks which are currently in
> place. Since you already have a PR open to HydromaticFileSetCheck
> support checkstyle 8, it seems like the changes aren't complex. Given
> that repo is also Apache licensed, wee could always pull the relevant
> code into Calcite since it's fairly small.
> --
> Michael Mior
> mm...@apache.org
> 
> Le mer. 4 déc. 2019 à 15:04, Vladimir Sitnikov
>  a écrit :
>> 
>> Michael>I didn't interpret Jullian's -1 as a veto
>> 
>>> https://www.apache.org/foundation/voting.html#Veto
>>> A code-modification proposal may be stopped dead in its tracks by a -1
>> vote by a qualified voter.
>>> This constitutes a veto, and it cannot be overruled nor overridden by
>> anyone.
>>> Vetos stand until and unless withdrawn by their casters.
>> 
>> Apparently that is a veto. What else could it be? However, it is "invalid
>> and has no weight".
>> 
>> I'm open to new opinions, however, I'm sure I have provided enough reasons
>> to just commit the PR and move forward.
>> 
>> Note: the only reason I started this discussion is I saw Julian's comments
>> in PR and in the JIRA, so I wanted to gather opinions.
>> Frankly speaking, I see nothing to discuss here.
>> 
>> Here's a recent (created 25m ago) PR by Andrei:
>> https://github.com/apache/calcite/pull/1632
>> Apparently, it failed with
>> [ant:checkstyle] [ERROR]
>> D:\a\calcite\calcite\core\src\main\java\org\apache\calcite\util\Sources.java:108:
>> Open parentheses exceed closes by 2 or more [HydromaticFileSet]
>> https://github.com/apache/calcite/pull/1632/checks#step:4:309
>> 
>> Here's how the new error would look like after HydromaticFileSetCheck is
>> dropped:
>> 
>>> The following files had format violations:
>>  core/src/main/java/org/apache/calcite/util/Sources.java
>>  @@ -105,7 +105,8 @@
>>   }
>> 
>>   private·UnsupportedOperationException·unsupported()·{
>> 
>> -··return·new·UnsupportedOperationException(String.format(Locale.ROOT,
>>  +··return·new·UnsupportedOperationException(
>>  +··String.format(Locale.ROOT,
>>   ··"Invalid·operation·for·'%s'·protocol",·protocol()));
>>   }
>> 
>> I find this error to be way better than "by 2 or more".
>> 
>> Vladimir

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Michael Mior]

I'll leave Julian to clarify whether or not the -1 was intended to be
a veto, but as you noted, Julian expressed some concern in the PR and
the associated JIRA. If we are voting, I'm currently -0 because I
haven't found checkstyle violations to be significant problem for me
and I don't like the idea of losing checks which are currently in
place. Since you already have a PR open to HydromaticFileSetCheck
support checkstyle 8, it seems like the changes aren't complex. Given
that repo is also Apache licensed, wee could always pull the relevant
code into Calcite since it's fairly small.
--
Michael Mior
mm...@apache.org

Le mer. 4 déc. 2019 à 15:04, Vladimir Sitnikov
 a écrit :
>
> Michael>I didn't interpret Jullian's -1 as a veto
>
> > https://www.apache.org/foundation/voting.html#Veto
> > A code-modification proposal may be stopped dead in its tracks by a -1
> vote by a qualified voter.
> > This constitutes a veto, and it cannot be overruled nor overridden by
> anyone.
> > Vetos stand until and unless withdrawn by their casters.
>
> Apparently that is a veto. What else could it be? However, it is "invalid
> and has no weight".
>
> I'm open to new opinions, however, I'm sure I have provided enough reasons
> to just commit the PR and move forward.
>
> Note: the only reason I started this discussion is I saw Julian's comments
> in PR and in the JIRA, so I wanted to gather opinions.
> Frankly speaking, I see nothing to discuss here.
>
> Here's a recent (created 25m ago) PR by Andrei:
> https://github.com/apache/calcite/pull/1632
> Apparently, it failed with
> [ant:checkstyle] [ERROR]
> D:\a\calcite\calcite\core\src\main\java\org\apache\calcite\util\Sources.java:108:
> Open parentheses exceed closes by 2 or more [HydromaticFileSet]
> https://github.com/apache/calcite/pull/1632/checks#step:4:309
>
> Here's how the new error would look like after HydromaticFileSetCheck is
> dropped:
>
> > The following files had format violations:
>   core/src/main/java/org/apache/calcite/util/Sources.java
>   @@ -105,7 +105,8 @@
>}
>
>private·UnsupportedOperationException·unsupported()·{
>
> -··return·new·UnsupportedOperationException(String.format(Locale.ROOT,
>   +··return·new·UnsupportedOperationException(
>   +··String.format(Locale.ROOT,
>··"Invalid·operation·for·'%s'·protocol",·protocol()));
>}
>
> I find this error to be way better than "by 2 or more".
>
> Vladimir

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Stamatis Zampetakis]

I'm not following many Apache projects but in those that I do I don't see
votes for code modifications very often. There is always some tension
whenever somebody calls for a vote so personally I would prefer if could go
without. I'm sure Vladimir did it with good intentions so that the
discussion moves a bit faster unlike other times where people may be more
passive.

Now regarding the proposal what I understand is that it solves a few
problems without any significant loss of functionality so it seems like a
reasonable change. One thing that I noticed is that before the plugin was
mainly maintained by Julian while after the PR it is the duty of all of us
since it becomes part of our code base. I think this is positive in the
sense that it would alleviate some weight from the shoulders of Julian.
One thing not so great is the modification of every file of the project
(due to the removal of the last line) but it seems possible to avoid it if
people have objections (I do not).

Having said that I don't really know much about what HydromaticFileSetCheck
was doing so I'm leaving the decision to those who are really familiar with
it.


On Wed, Dec 4, 2019 at 9:04 PM Vladimir Sitnikov <
sitnikov.vladi...@gmail.com> wrote:

> Michael>I didn't interpret Jullian's -1 as a veto
>
> > https://www.apache.org/foundation/voting.html#Veto
> > A code-modification proposal may be stopped dead in its tracks by a -1
> vote by a qualified voter.
> > This constitutes a veto, and it cannot be overruled nor overridden by
> anyone.
> > Vetos stand until and unless withdrawn by their casters.
>
> Apparently that is a veto. What else could it be? However, it is "invalid
> and has no weight".
>
> I'm open to new opinions, however, I'm sure I have provided enough reasons
> to just commit the PR and move forward.
>
> Note: the only reason I started this discussion is I saw Julian's comments
> in PR and in the JIRA, so I wanted to gather opinions.
> Frankly speaking, I see nothing to discuss here.
>
> Here's a recent (created 25m ago) PR by Andrei:
> https://github.com/apache/calcite/pull/1632
> Apparently, it failed with
> [ant:checkstyle] [ERROR]
>
> D:\a\calcite\calcite\core\src\main\java\org\apache\calcite\util\Sources.java:108:
> Open parentheses exceed closes by 2 or more [HydromaticFileSet]
> https://github.com/apache/calcite/pull/1632/checks#step:4:309
>
> Here's how the new error would look like after HydromaticFileSetCheck is
> dropped:
>
> > The following files had format violations:
>   core/src/main/java/org/apache/calcite/util/Sources.java
>   @@ -105,7 +105,8 @@
>}
>
>private·UnsupportedOperationException·unsupported()·{
>
> -··return·new·UnsupportedOperationException(String.format(Locale.ROOT,
>   +··return·new·UnsupportedOperationException(
>   +··String.format(Locale.ROOT,
>··"Invalid·operation·for·'%s'·protocol",·protocol()));
>}
>
> I find this error to be way better than "by 2 or more".
>
> Vladimir
>

Re: lex parameter does not get taken into account for views

[Julian Hyde]

As I remarked in the Jira, we don’t want views to be expanded according to the 
rules of the current connection. Views belong to the schema, and the schema is 
shared among many connections, which may have different lex parameters.

> On Dec 1, 2019, at 10:06 PM, XING JIN  wrote:
> 
> Filed a JIRA: https://issues.apache.org/jira/browse/CALCITE-3549
> 
> Danny Chan  于2019年12月2日周一 上午9:06写道：
> 
>> Dear Erin ~
>> 
>> You are right, Calcite now always hard code the parser config for JDBC
>> query[1], that means, Lex config for view expanding is not supported yet,
>> can you log a issue and probably fire a patch to support that?
>> 
>> [1]
>> https://github.com/apache/calcite/blob/ab136b5f76a4cb951e847fcba6b414c5e80dbbe6/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java#L385
>> 
>> Best,
>> Danny Chan
>> 在 2019年12月2日 +0800 AM4:19，Erin Drummond ，写道：
>>> Hello,
>>> 
>>> I am currently attempting to test out the views feature of Calcite.
>>> 
>>> I am using a connection string like so:
>>> jdbc:calcite:lex=JAVA;model=inline:
>>> 
>>> I have created a JdbcSchema that is attached to an actual database - lets
>>> call it "t1". I can successfully run the query "select * from t1.table
>>> limit 10"
>>> 
>>> I then created another schema, defined like so:
>>> {
>>> "name": "viewtest",
>>> "tables": [
>>> { "name": "test_view", "type": "view", "sql": "select * from
>>> t1.table limit 10" }
>>> ]
>>> }
>>> 
>>> The idea is to set up connections to a bunch of physical data sources and
>>> then have a "views" schema that just contains views over them.
>>> 
>>> However, when attempting to run the query "select * from
>>> viewtest.test_view", I get the following exception:
>>> 
>>> StatementCallback; uncategorized SQLException for SQL [select * from
>>> viewtest.test_view]; SQL state [null]; error code [0];
>>> Error while executing SQL \"select * from viewtest.test_view\": From line
>>> 1, column 15 to line 1, column 32: Object 'T1' not found; did you mean
>> 't1'?
>>> 
>>> I can clearly see that lex=JAVA is not being used for the view and the
>>> default lex=ORACLE is being used. If I change the view SQL to 'select *
>>> from "t1".table limit 10' it works - but I don't want to have to quote
>> the
>>> identifiers oracle style, I would like the lex=JAVA that I specified on
>> the
>>> connection string to flow down to views as well.
>>> 
>>> What am I missing here?
>>> 
>>> Cheers,
>>> Erin
>> 

Re: [DISCUSS] Towards Avatica 1.16.0

[Francis Chuang]

Hi everyone,

Just a quick reminder that I am making rc0 available for voting 
tomorrow. If you need to get any change in, or if the time-frame is 
inconvenient, please let me know.


Francis

On 2/12/2019 9:07 am, Francis Chuang wrote:
The Avatica 1.16.0 release seems to be a bit overdue. I plan to make rc0 
available for voting on Friday morning (the 6th of December) Australian 
Eastern Standard time (probably Thursday in some parts of Europe and 
America).


If there are any changes you would like to see in this release, please 
try to get them in. If committers could review and merge some of the 
open PRs[1], that would be highly appreciated too!


If the proposed time frame does not work for you, please let me know.

Francis

[1] https://github.com/apache/calcite-avatica/pulls

On 10/10/2019 10:26 am, Francis Chuang wrote:

Hey everyone,

It's been around 5 months since the last Avatica release. There's been 
a couple of commits to the code-base since and I'd like to use this as 
an opportunity to get a few more PRs in and make a release available 
for voting. I am happy to be release manager for this release.


In terms of PRs I am hoping for the following to be merged for this 
release:


- CALCITE-3384: Support Kerberos using SPNEGO over HTTPS - 
https://github.com/apache/calcite-avatica/pull/113 (Josh is reviewing)


- CALCITE-333: Add pluggable frame size limits - 
https://issues.apache.org/jira/browse/CALCITE- (looking for a 
reviewer)


- CALCITE-3199: Fix unixDateCeil and unixDateFloor - 
https://github.com/apache/calcite-avatica/pull/109 (looking for a 
reviewer)


- CALCITE-3162: Reading arrays from Calcite through JdbcMeta throws 
exception - https://github.com/apache/calcite-avatica/pull/106 
(looking for a reviewer)


- CALCITE-3163: Mapping types in AbstractCursor does not adhere to 
JDBC spec - https://github.com/apache/calcite-avatica/pull/105 
(looking for a reviewer)


- CALCITE-3158: Move build system to Gradle - 
https://github.com/apache/calcite-avatica/pull/104 (Vladimir has put a 
lot of effort into this. Can we get some consensus as to whether this 
should to be merged?)


- CALCITE-3078: Duplicate code for lastDay in Calcite and Avatica - 
https://github.com/apache/calcite-avatica/pull/98 (looking for a 
reviewer)


- No Jira: Support UNIX timestamp to string date - 
https://github.com/apache/calcite-avatica/pull/96 (can someone confirm 
if these changes are valid or should the PR be closed?)


- CALCITE-2704: Avoid using ISO-8859-1 to parse request in JSONHandler 
- https://github.com/apache/calcite-avatica/pull/85 (Someone posted 
some tests in the issue comments. Can someone please add the tests, 
review and merge?)


- CALCITE-1806: Add Spark JDBC tests - 
https://github.com/apache/calcite-avatica/pull/28 (Kevin, any chance 
you can take another look at this?)


If you have some spare cycles for Avatica, please pick up some of the 
PRs in this list, it would be very much appreciated!


Francis

Re: Release managers

[Julian Hyde]

Sure, take 1.24.

By the way, if people are eager to be release managers, we might need some for 
Avatica release-train.



> On Dec 2, 2019, at 5:11 PM, Chunwei Lei  wrote:
> 
> It's great to know that committers can be release manager.
> I volunteer to be the release manager if there is a chance.
> 
> BTW, If Julian doesn't mind, I would like to take 1.24 ~~
> 
> 
> Best,
> Chunwei
> 
> 
> On Tue, Dec 3, 2019 at 5:43 AM Francis Chuang 
> wrote:
> 
>> Just a quick note regarding the move to Gradle as the build tool for
>> release. Thanks to Vladimir, the release process is almost entirely
>> automated and the rc can be built and automatically uploaded to ASF's
>> servers for voting using just one command: ./gradlew prepareVote -Prc=0
>> -Pasf This builds the artifacts, signs them and uploads them to
>> dist.apache.org.
>> 
>> If the RM is a committer, then I think they should do a dry-run to test
>> the build, but not upload it. The asflike-release-environment[1] should
>> be used to try uploading the release to a test environment: ./gradlew
>> prepareVote -Prc=0.
>> 
>> Once everything looks good, a PMC member should build and upload the
>> signed release using ./gradlew prepareVote -Prc=0 -Pasf and forward the
>> vote email to the RM.
>> 
>> In the past, maven only built the artifacts and left the uploading of
>> the files as a manual exercise to the RM, so the process has changed
>> slightly this time.
>> 
>> Francis
>> 
>> [1] https://github.com/vlsi/asflike-release-environment
>> 
>> On 3/12/2019 6:03 am, Julian Hyde wrote:
>>> I volunteer to do 1.24.
>>> 
>>> There’s one part of the release process that only a PMC member can do -
>> namely, to sign the artifacts. But that’s only a small part of the process,
>> and you can easily get a PMC member to do it for you. A much larger part of
>> the process is the herding of cats (committers, bugs, pull requests,
>> release notes). So, yes, a committer can definitely be a release manager.
>>> 
>>> How does the PMC decide which committers to promote to PMC members? We
>> are looking for people who help out around the project, going above and
>> beyond the basic needs of each task to make the project a better place.  If
>> you are a committer, helping with the release process is a good way to earn
>> merit.
>>> 
>>> Julian
>>> 
>>> 
 On Dec 2, 2019, at 10:48 AM, Stamatis Zampetakis 
>> wrote:
 
 Many thanks Haisheng and Danny for stepping up! I added you to the list.
 There are two spots left, if nobody else comes up I will take one of
>> them!
 
 Release Target date Release manager
 === === ===
 1.192019-03Kevin
 1.202019-06Michael
 1.212019-09Stamatis
 === === ===
 1.222019-12Andrei
 1.232020-02Haisheng
 1.242020-04Julian
 1.252020-06Danny
 1.262020-08
 
 
 
 On Sat, Nov 30, 2019 at 9:52 AM Danny Chan 
>> wrote:
 
> BTW,
> I can volunteer to be the release manager for v1.25 or v1.26.
> 
> Best,
> Danny Chan
> 在 2019年11月30日 +0800 PM2:13，dev@calcite.apache.org，写道：
>> 
>> I can volunteer to be the release manager for v1.23 or v1.24.
> 
>>> 
>> 

Re: [DISCUSS] Towards Calcite 1.22

[Julian Hyde]

I don’t mind whether the release happens in December or January, but either 
way, let’s start burning down the backlog of PRs now.


> On Dec 2, 2019, at 11:43 PM, Enrico Olivelli  wrote:
> 
> Andrei
> 
> Il mar 3 dic 2019, 08:21 Rui Wang  > ha scritto:
> 
>> Thank you for this notification.
>> 
>> Please try to make CALCITE-3272[1] into 1.22 (change the fix version to
>> 1.22 already).
>> 
>> 
>> [1]: https://github.com/apache/calcite/pull/1587
>> 
>> -Rui
>> 
>> On Mon, Dec 2, 2019 at 6:35 PM Chunwei Lei  wrote:
>> 
>>> Thank you for your work, Anderi.
>>> 
>>> Let's get CALCITE-1581[1] into 1.22.
>>> 
>>> +1 for release at early-mid January '20 (to have more time to review
>> prs).
>>> 
>>> 
>>> [1] https://github.com/apache/calcite/pull/1138
>>> 
>>> 
>>> Best,
>>> Chunwei
>>> 
>>> 
>>> On Tue, Dec 3, 2019 at 8:02 AM Andrei Sereda  wrote:
>>> 
 Hello,
 
 Calcite 1.21 was released about 3 months ago (2019-09) and it is time
>> to
 start preparation for 1.22.
 
 Current open issues and pull requests can be seen in [1] and [2]. There
>>> are
 many PRs left from previous releases and it would be nice to review as
>>> many
 as possible. Please change "fix version" in JIRA to 1.22 if you would
>>> like
 the contribution be considered for this release. It is also helpful to
>>> mark
 PR with "LGTM-will-merge-soon" label so other contributors are aware of
 your review.
 
 Committers please go over existing PRs and try to prioritize / finalize
 them. Also let me know which changes (in your opinion) are ready or
>>> should
 be considered for this release. Don't forget that current policy of
 frequent releases allows better work scheduling without blocking
>> existing
 release plan.
 
 In terms of dates, let's agree on release time frame late December '19
>> or
 early-mid January '20 ?
>> 
> 
> I (from HerdDB community) would prefer late December if possible, as we are
> stuck to an older 1.19 version of Calcite.
> Current Calcite master is is great shape from our point of view
> 
> Thanks for driving this
> Enrico
> 
> 
> 
> 
> 
>>> 
 Let me know if I have missed anything or if current plan is
>> inconvenient.
 
 Thanks,
 Andrei.
 
 [1]
 
>>> 
>> https://issues.apache.org/jira/secure/Dashboard.jspa?selectPageId=12333950 
>> 
 [2] https://github.com/apache/calcite/pulls 
 

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Vladimir Sitnikov]

Michael>I didn't interpret Jullian's -1 as a veto

> https://www.apache.org/foundation/voting.html#Veto
> A code-modification proposal may be stopped dead in its tracks by a -1
vote by a qualified voter.
> This constitutes a veto, and it cannot be overruled nor overridden by
anyone.
> Vetos stand until and unless withdrawn by their casters.

Apparently that is a veto. What else could it be? However, it is "invalid
and has no weight".

I'm open to new opinions, however, I'm sure I have provided enough reasons
to just commit the PR and move forward.

Note: the only reason I started this discussion is I saw Julian's comments
in PR and in the JIRA, so I wanted to gather opinions.
Frankly speaking, I see nothing to discuss here.

Here's a recent (created 25m ago) PR by Andrei:
https://github.com/apache/calcite/pull/1632
Apparently, it failed with
[ant:checkstyle] [ERROR]
D:\a\calcite\calcite\core\src\main\java\org\apache\calcite\util\Sources.java:108:
Open parentheses exceed closes by 2 or more [HydromaticFileSet]
https://github.com/apache/calcite/pull/1632/checks#step:4:309

Here's how the new error would look like after HydromaticFileSetCheck is
dropped:

> The following files had format violations:
  core/src/main/java/org/apache/calcite/util/Sources.java
  @@ -105,7 +105,8 @@
   }

   private·UnsupportedOperationException·unsupported()·{

-··return·new·UnsupportedOperationException(String.format(Locale.ROOT,
  +··return·new·UnsupportedOperationException(
  +··String.format(Locale.ROOT,
   ··"Invalid·operation·for·'%s'·protocol",·protocol()));
   }

I find this error to be way better than "by 2 or more".

Vladimir

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Michael Mior]

I didn't interpret Jullian's -1 as a veto. I agree that we generally
don't need to vote on code changes. I don't believe we ever voted on
the Gradle migration and IMO that was a much moree significant change.
--
Michael Mior
mm...@apache.org


Le mer. 4 déc. 2019 à 14:28, Vladimir Sitnikov
 a écrit :
>
> Julian>-1
>
> I see your pain, however, please remember:
>
> ASF> https://www.apache.org/foundation/voting.html#Veto
> ASF> A veto without a justification is invalid and has no weight
>
> The PR is a clear improvement, and we can always make it better (e.g. by
> committing more code).
> We even can roll it back if it turns out to be not that good.
>
> Vladimir

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Vladimir Sitnikov]

Julian>-1

I see your pain, however, please remember:

ASF> https://www.apache.org/foundation/voting.html#Veto
ASF> A veto without a justification is invalid and has no weight

The PR is a clear improvement, and we can always make it better (e.g. by
committing more code).
We even can roll it back if it turns out to be not that good.

Vladimir

Re: [VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Julian Hyde]

It’s unusual to vote on a code change. We could just have a discussion, and 
reach consensus about whether the PR is a net benefit.

-1



> On Dec 4, 2019, at 2:30 AM, Vladimir Sitnikov  
> wrote:
> 
> Hi,
> 
> I suggest we drop HydromaticFileSetCheck from the Checkstyle configuration.
> It was a great journey (thanks Julian for HydromaticFileSetCheck), however,
> lots of tools have improved since then, so we could upgrade to the better
> tooling now.
> 
> [ ] +1, drop HydromaticFileSetCheck, just do it
> [ ] -1, do not drop it because...
> 
> The vote is open for 120 hours (till 2019-12-09 11:00:00 UTC)
> 
> Motivation:
> * Current Checkstyle 7.x has concurrency issues, and it produces wrong
> results when multiple modules are checked concurrently
> * HydromaticFileSetCheck blocks upgrade to Checkstyle 8
> * HydromaticFileSetCheck seems to cause issues in Eclipse (see CALCITE-1127)
> * HydromaticFileSetCheck is not supported
> : the latest commit was 3-4 years
> ago, trivial issues are not resolved since July, trivial pull request
> receives no comments for more than 15 days
> * HydromaticFileSetCheck produces hard to understand messages. I guess many
> of you have faced "Open parentheses exceed closes by 2 or more
> [HydromaticFileSet]" issue, and I guess you it took you a lot to understand
> what the error really means.
> * HydromaticFileSetCheck duplicates logic which is already implemented
> elsewhere (e.g. fail on tab characters, end with newline)
> * HydromaticFileSetCheck produces errors when automatic correction is
> possible. For instance, "@Override should not be on its own line"
> * "// End ..." file footer is redundant, and sometimes it is misleading
> (e.g. // End Parser.jj is retained even in the generated parser which makes
> no sense).
> 
> Solution: https://github.com/apache/calcite/pull/1625
> Please check the PR description and a showcase for parenthesis
>  for
> more info.
> In a nutshell, it implements automatic fixes, so the build produces easier
> to understand errors, and the fixes could be applied automatically.
> 
> The PR discovers a few violations (~10) that were never detected before.
> For instance (even though it looks like a diff, it is a copy-paste of the
> build script output where it complains on the wrong formatting and it
> suggests the fix):
> 
> core/src/main/java/org/apache/calcite/rel/rules/AggregateValuesRule.java
>  -literals.add((RexLiteral)·rexBuilder.makeLiteral(
>  +literals.add(
>  +(RexLiteral)·rexBuilder.makeLiteral(
>  BigDecimal.ZERO,·aggregateCall.getType(),·false));
> 
> kafka/src/main/java/org/apache/calcite/adapter/kafka/KafkaTableFactory.java
> 
> if·(operand.containsKey(KafkaTableConstants.SCHEMA_CONSUMER_PARAMS))·{
> 
> -··tableOptionBuilder.setConsumerParams((Map)·operand.get(
>  +··tableOptionBuilder.setConsumerParams(
>  +··(Map)·operand.get(
>   ··KafkaTableConstants.SCHEMA_CONSUMER_PARAMS));
>   }
> 
> I'll fix the violations as a part of the PR as well. Note: in the both
> cases I've provided above the autofix is suboptimal,
> however, I guess does convey the intended formatting.
> 
> Vladimir

[jira] [Created] (CALCITE-3569) IndexOutOfBoundsException when pushing FALSE filter to view

[Chunwei Lei (Jira)]

Chunwei Lei created CALCITE-3569:


 Summary: IndexOutOfBoundsException when pushing FALSE filter to 
view
 Key: CALCITE-3569
 URL: https://issues.apache.org/jira/browse/CALCITE-3569
 Project: Calcite
  Issue Type: Improvement
  Components: core
Affects Versions: 1.21.0
Reporter: Chunwei Lei
Assignee: Chunwei Lei


It can be reproduced by the following test.
{code:java}
// MaterializationTest.java
@Test public void testAggregate7() {
  try (TryThreadLocal.Memo ignored = Prepare.THREAD_TRIM.push(true)) {
MaterializationService.setThreadLocal();
CalciteAssert.that()
.withMaterializations(
HR_FKUK_MODEL,
"m0",
"select 11 as \"empno\", 22 as \"sal\", count(*) from \"emps\" 
group by 11, 22")
.query(
"select * from\n"
+ "(select 11 as \"empno\", 22 as \"sal\", count(*)\n"
+ "from \"emps\" group by 11, 22) tmp\n"
+ "where \"sal\" = 33")
.enableMaterializations(true)
.explainContains("EnumerableValues(tuples=[[]])");
  }
}
{code}



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

[jira] [Created] (CALCITE-3568) BigQuery SQL dialect doesn't support nested aggregates.

[Divyanshu Srivastava (Jira)]

Divyanshu Srivastava created CALCITE-3568:
-

 Summary: BigQuery SQL dialect doesn't support nested aggregates.
 Key: CALCITE-3568
 URL: https://issues.apache.org/jira/browse/CALCITE-3568
 Project: Calcite
  Issue Type: Improvement
  Components: core
Reporter: Divyanshu Srivastava
 Fix For: 1.22.0


The BigQuerySqlDialect does not support nested aggregates, hence overriding the 
supportsNestedAggregations() method in it.



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

[VOTE] [CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[Vladimir Sitnikov]

Hi,

I suggest we drop HydromaticFileSetCheck from the Checkstyle configuration.
It was a great journey (thanks Julian for HydromaticFileSetCheck), however,
lots of tools have improved since then, so we could upgrade to the better
tooling now.

[ ] +1, drop HydromaticFileSetCheck, just do it
[ ] -1, do not drop it because...

The vote is open for 120 hours (till 2019-12-09 11:00:00 UTC)

Motivation:
* Current Checkstyle 7.x has concurrency issues, and it produces wrong
results when multiple modules are checked concurrently
* HydromaticFileSetCheck blocks upgrade to Checkstyle 8
* HydromaticFileSetCheck seems to cause issues in Eclipse (see CALCITE-1127)
* HydromaticFileSetCheck is not supported
: the latest commit was 3-4 years
ago, trivial issues are not resolved since July, trivial pull request
receives no comments for more than 15 days
* HydromaticFileSetCheck produces hard to understand messages. I guess many
of you have faced "Open parentheses exceed closes by 2 or more
[HydromaticFileSet]" issue, and I guess you it took you a lot to understand
what the error really means.
* HydromaticFileSetCheck duplicates logic which is already implemented
elsewhere (e.g. fail on tab characters, end with newline)
* HydromaticFileSetCheck produces errors when automatic correction is
possible. For instance, "@Override should not be on its own line"
* "// End ..." file footer is redundant, and sometimes it is misleading
(e.g. // End Parser.jj is retained even in the generated parser which makes
no sense).

Solution: https://github.com/apache/calcite/pull/1625
Please check the PR description and a showcase for parenthesis
 for
more info.
In a nutshell, it implements automatic fixes, so the build produces easier
to understand errors, and the fixes could be applied automatically.

The PR discovers a few violations (~10) that were never detected before.
For instance (even though it looks like a diff, it is a copy-paste of the
build script output where it complains on the wrong formatting and it
suggests the fix):

core/src/main/java/org/apache/calcite/rel/rules/AggregateValuesRule.java
  -literals.add((RexLiteral)·rexBuilder.makeLiteral(
  +literals.add(
  +(RexLiteral)·rexBuilder.makeLiteral(
  BigDecimal.ZERO,·aggregateCall.getType(),·false));

kafka/src/main/java/org/apache/calcite/adapter/kafka/KafkaTableFactory.java

 if·(operand.containsKey(KafkaTableConstants.SCHEMA_CONSUMER_PARAMS))·{

-··tableOptionBuilder.setConsumerParams((Map)·operand.get(
  +··tableOptionBuilder.setConsumerParams(
  +··(Map)·operand.get(
   ··KafkaTableConstants.SCHEMA_CONSUMER_PARAMS));
   }

I'll fix the violations as a part of the PR as well. Note: in the both
cases I've provided above the autofix is suboptimal,
however, I guess does convey the intended formatting.

Vladimir

[jira] [Created] (CALCITE-3567) UNNEST(RECORDTYPE(MAP)) not supported

[Wang Yanlin (Jira)]

Wang Yanlin created CALCITE-3567:


 Summary: UNNEST(RECORDTYPE(MAP)) not supported
 Key: CALCITE-3567
 URL: https://issues.apache.org/jira/browse/CALCITE-3567
 Project: Calcite
  Issue Type: Bug
Reporter: Wang Yanlin


UNNEST(RECORDTYPE(ARRAY)) and  UNNEST(RECORDTYPE(MULTISET)) works well, but 
UNNEST(RECORDTYPE(MAP)) is not supported.

{code:java}
// JdbcTest
@Test public void testUnnestRecordType() {
// unnest(RecordType(Array))
CalciteAssert.that()
.query("select * from unnest\n"
+ "(select t.x from (values array[10, 20], array[30, 40]) as 
t(x))\n"
+ " with ordinality as t(a, o)")
.returnsUnordered("A=10; O=1", "A=20; O=2",
"A=30; O=1", "A=40; O=2");

// unnest(RecordType(Multiset))
CalciteAssert.that()
.query("select * from unnest\n"
+ "(select t.x from (values multiset[10, 20], array[30, 40]) as 
t(x))\n"
+ " with ordinality as t(a, o)")
.returnsUnordered("A=10; O=1", "A=20; O=2",
"A=30; O=1", "A=40; O=2");

// unnest(RecordType(Map))
CalciteAssert.that()
.query("select * from unnest\n"
+ "(select t.x from (values map['a', 20, 'b', 30], map['c', 40]) as 
t(x))\n"
+ " with ordinality as t(a, b, o)")
.returnsUnordered("A=a; B=20; O=1", "A=B; B=30; O=2",
"A=c; B=40; O=1");
  }
{code}

In the case, test for *unnest(RecordType(Array))* and 
*unnest(RecordType(Multiset))* success, but got exception for 
*unnest(RecordType(Map))*.

For the first two test of *unnest(RecordType(Array))* and 
*unnest(RecordType(Multiset))*, the relnode tree with type is like this

{noformat}
EnumerableUncollect(withOrdinality=[true]), type = RecordType(INTEGER EXPR$0, 
INTEGER ORDINALITY)
  EnumerableUnion(all=[true]), type = RecordType(INTEGER ARRAY EXPR$0)
EnumerableCalc(expr#0=[{inputs}], expr#1=[10], expr#2=[20], 
expr#3=[ARRAY($t1, $t2)], EXPR$0=[$t3]), type = RecordType(INTEGER ARRAY EXPR$0)
  EnumerableValues(tuples=[[{ 0 }]]), type = RecordType(INTEGER ZERO)
EnumerableCalc(expr#0=[{inputs}], expr#1=[30], expr#2=[40], 
expr#3=[ARRAY($t1, $t2)], EXPR$0=[$t3]), type = RecordType(INTEGER ARRAY EXPR$0)
  EnumerableValues(tuples=[[{ 0 }]]), type = RecordType(INTEGER ZERO)

EnumerableUncollect(withOrdinality=[true]), type = RecordType(INTEGER EXPR$0, 
INTEGER ORDINALITY)
  EnumerableUnion(all=[true]), type = RecordType(INTEGER ARRAY EXPR$0)
EnumerableCalc(expr#0=[{inputs}], expr#1=[$SLICE($t0)], EXPR$0=[$t1]), type 
= RecordType(INTEGER MULTISET EXPR$0)
  EnumerableCollect(field=[EXPR$0]), type = RecordType(RecordType(INTEGER 
ROW_VALUE) MULTISET EXPR$0)
EnumerableValues(tuples=[[{ 10 }, { 20 }]]), type = RecordType(INTEGER 
ROW_VALUE)
EnumerableCalc(expr#0=[{inputs}], expr#1=[30], expr#2=[40], 
expr#3=[ARRAY($t1, $t2)], EXPR$0=[$t3]), type = RecordType(INTEGER ARRAY EXPR$0)
  EnumerableValues(tuples=[[{ 0 }]]), type = RecordType(INTEGER ZERO)
{noformat}



Part of the stacktrace of the exception is:

{code:java}

Cannot apply 'UNNEST' to arguments of type 'UNNEST()'. Supported form(s): 'UNNEST()'
'UNNEST()'
'UNNEST()'
UNNEST()
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at 
org.apache.calcite.runtime.Resources$ExInstWithCause.ex(Resources.java:463)
at org.apache.calcite.sql.SqlUtil.newContextException(SqlUtil.java:839)
at org.apache.calcite.sql.SqlUtil.newContextException(SqlUtil.java:824)
at 
org.apache.calcite.sql.validate.SqlValidatorImpl.newValidationError(SqlValidatorImpl.java:4905)
at 
org.apache.calcite.sql.SqlCallBinding.newValidationSignatureError(SqlCallBinding.java:280)
at 
org.apache.calcite.sql.type.CompositeOperandTypeChecker.checkOperandTypes(CompositeOperandTypeChecker.java:261)
at 
org.apache.calcite.sql.SqlOperator.checkOperandTypes(SqlOperator.java:668)
at 
org.apache.calcite.sql.SqlOperator.validateOperands(SqlOperator.java:432)
at 
org.apache.calcite.sql.validate.UnnestNamespace.validateImpl(UnnestNamespace.java:66)
at 
org.apache.calcite.sql.validate.AbstractNamespace.validate(AbstractNamespace.java:84)
at 
org.apache.calcite.sql.validate.SqlValidatorImpl.validateNamespace(SqlValidatorImpl.java:1009)
at 
org.apache.calcite.sql.validate.SqlValidatorImpl.validateQuery(SqlValidatorImpl.java:969)
at 
org.apache.calcite.sql.validate.SqlValidatorImpl.validateUnnest(SqlValidatorImpl.java:3171)
at 

[jira] [Created] (CALCITE-3566) Support EnumerableIntersect and EnumerableMinus convert to Logical

[xzh_dz (Jira)]

xzh_dz created CALCITE-3566:
---

 Summary: Support EnumerableIntersect and EnumerableMinus convert 
to Logical
 Key: CALCITE-3566
 URL: https://issues.apache.org/jira/browse/CALCITE-3566
 Project: Calcite
  Issue Type: Wish
Reporter: xzh_dz


In ToLogicalConverter, EnumerableIntersect and EnumerableMinus can not convert 
to Logical currently, this PR supports this transformation.

 



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

[jira] [Created] (CALCITE-3565) Explicitly cast assignable operand types to decimal for udf

[Feng Zhu (Jira)]

Feng Zhu created CALCITE-3565:
-

 Summary: Explicitly cast assignable operand types to decimal for 
udf
 Key: CALCITE-3565
 URL: https://issues.apache.org/jira/browse/CALCITE-3565
 Project: Calcite
  Issue Type: Sub-task
Reporter: Feng Zhu
Assignee: Feng Zhu






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

[jira] [Created] (CALCITE-3564) Make operand type check accurate for some functions in validation phase

[Feng Zhu (Jira)]

Feng Zhu created CALCITE-3564:
-

 Summary: Make operand type check accurate for some functions in 
validation phase
 Key: CALCITE-3564
 URL: https://issues.apache.org/jira/browse/CALCITE-3564
 Project: Calcite
  Issue Type: Sub-task
Reporter: Feng Zhu






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

[jira] [Created] (CALCITE-3563) Convert function operand type to match implementation if necessary in runtime

[Feng Zhu (Jira)]

Feng Zhu created CALCITE-3563:
-

 Summary: Convert function operand type to match implementation if 
necessary in runtime
 Key: CALCITE-3563
 URL: https://issues.apache.org/jira/browse/CALCITE-3563
 Project: Calcite
  Issue Type: Sub-task
  Components: core
Reporter: Feng Zhu






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