[jira] [Commented] (CALCITE-4256) RexSimplify should not simplify P AND P to P, if it contains a call to RAND or RAND_INTEGER
[
https://issues.apache.org/jira/browse/CALCITE-4256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195833#comment-17195833
]
Chunwei Lei commented on CALCITE-4256:
--
Agree with [~danny0405]. In our system, we treat SqlRandFunction as a
non-deterministic function.
> RexSimplify should not simplify P AND P to P, if it contains a call to RAND
> or RAND_INTEGER
> ---
>
> Key: CALCITE-4256
> URL: https://issues.apache.org/jira/browse/CALCITE-4256
> Project: Calcite
> Issue Type: Bug
>Reporter: Thomas Rebele
>Priority: Major
>
> Example: RAND_INTEGER() = 1 AND RAND_INTEGER() = 1 is false with a higher
> probability than RAND_INTEGER() = 1
> Here a test case for RexProgramTest:
> {code}
> @Test void testSimplifyRandomAnd() {
> checkSimplifyUnchanged(
> and(
> eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
> literal(1)),
> eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
> literal(1))
> ));
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4256) RexSimplify should not simplify P AND P to P, if it contains a call to RAND or RAND_INTEGER
[
https://issues.apache.org/jira/browse/CALCITE-4256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195816#comment-17195816
]
Danny Chen commented on CALCITE-4256:
-
It might be wrong that {{SqlRandFunction.isDeterministic}} returns true.
> RexSimplify should not simplify P AND P to P, if it contains a call to RAND
> or RAND_INTEGER
> ---
>
> Key: CALCITE-4256
> URL: https://issues.apache.org/jira/browse/CALCITE-4256
> Project: Calcite
> Issue Type: Bug
>Reporter: Thomas Rebele
>Priority: Major
>
> Example: RAND_INTEGER() = 1 AND RAND_INTEGER() = 1 is false with a higher
> probability than RAND_INTEGER() = 1
> Here a test case for RexProgramTest:
> {code}
> @Test void testSimplifyRandomAnd() {
> checkSimplifyUnchanged(
> and(
> eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
> literal(1)),
> eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
> literal(1))
> ));
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (CALCITE-4257) Allow CachingRelMetadataProvider.cache to be parameterized in the constructor
James Starr created CALCITE-4257: Summary: Allow CachingRelMetadataProvider.cache to be parameterized in the constructor Key: CALCITE-4257 URL: https://issues.apache.org/jira/browse/CALCITE-4257 Project: Calcite Issue Type: Improvement Components: core Reporter: James Starr My company currently has custom version of CachingRelMetadataProvider because we need to able to configure the type of Map it uses. It would be nice if it just took it as an argument. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (CALCITE-4256) RexSimplify should not simplify P AND P to P, if it contains a call to RAND or RAND_INTEGER
[
https://issues.apache.org/jira/browse/CALCITE-4256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195664#comment-17195664
]
Julian Hyde commented on CALCITE-4256:
--
Let's frame this in terms of whether the operator is deterministic. See
{{SqlOperator.isDeterministic()}}. The fix certainly should not reference
particular operators.
Related: {{SqlRandFunction.isDynamicFunction()}} is overridden to return true,
and I'm not sure whether that would be necessary if it were flagged
non-deterministic.
> RexSimplify should not simplify P AND P to P, if it contains a call to RAND
> or RAND_INTEGER
> ---
>
> Key: CALCITE-4256
> URL: https://issues.apache.org/jira/browse/CALCITE-4256
> Project: Calcite
> Issue Type: Bug
>Reporter: Thomas Rebele
>Priority: Major
>
> Example: RAND_INTEGER() = 1 AND RAND_INTEGER() = 1 is false with a higher
> probability than RAND_INTEGER() = 1
> Here a test case for RexProgramTest:
> {code}
> @Test void testSimplifyRandomAnd() {
> checkSimplifyUnchanged(
> and(
> eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
> literal(1)),
> eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
> literal(1))
> ));
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-2082) Do not store types or type factories inside operators
[
https://issues.apache.org/jira/browse/CALCITE-2082?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195656#comment-17195656
]
Julian Hyde commented on CALCITE-2082:
--
bq. was there a PR for this patch?
Yes, it was part of https://github.com/apache/calcite/pull/2111.
bq. Also, please notice that this is a breaking change,
Yes.
bq. Why we must now have a {{SqlOperandMetadata}}
This
[change|https://github.com/apache/calcite/commit/39cf82b8bf94b9240fe4592706d8eeb2952712f0#diff-9fe616c8f4434e9f6692853abf0bb778L607]
illustrates:
{noformat}
@@ -604,15 +604,20 @@ public static boolean matchRoutinesByParameterCount(
return (Iterator) Iterators.filter(
Iterators.filter(routines, SqlFunction.class),
function -> {
- List paramTypes = function.getParamTypes();
- if (paramTypes == null) {
+ if (Objects.requireNonNull(function).getOperandTypeChecker() == null
+ || !function.getOperandTypeChecker().isFixedParameters()) {
// no parameter information for builtins; keep for now,
// the type coerce will not work here.
return true;
}
+ final SqlOperandMetadata operandMetadata =
+ (SqlOperandMetadata) function.getOperandTypeChecker();
+ final List paramTypes =
+ operandMetadata.paramTypes(typeFactory);
final List permutedArgTypes;
if (argNames != null) {
-permutedArgTypes = permuteArgTypes(function, argNames, argTypes);
+final List paramNames = operandMetadata.paramNames();
+permutedArgTypes = permuteArgTypes(paramNames, argNames, argTypes);
if (permutedArgTypes == null) {
return false;
} {noformat}
Previously, we called {{function.getParamTypes()}}. How could the function
possibly know its own types if we do not support a type factory, and the
function was created before the current statement, and in fact before any type
factories were created?
Breaking the API was unavoidable - you must pass in a {{RelDataTypeFactory}} in
order to get a type, and the previous API had no way to pass a factory. Note
that you can get the number and names of parameters without passing a factory.
The new API makes a stronger distinction between UDFs and built-in functions.
UDFs' parameters are fixed in number and type, and have names. Built-ins may be
overloaded - that is, the parameters may vary in number and type - but the
parameters do not have names.
bq. My application had several UDFs with {{SqlOperandTypeChecker}}, and they
already complied with "Do not store types (RelDataType) or type factories
(RelDataTypeFactory) in SqlOperator instances"
How did you do this in your application? (Except if your UDFs had no
parameters.)
> Do not store types or type factories inside operators
> -
>
> Key: CALCITE-2082
> URL: https://issues.apache.org/jira/browse/CALCITE-2082
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
> Fix For: 1.26.0
>
>
> Do not store types (RelDataType) or type factories (RelDataTypeFactory) in
> SqlOperator instances.
> *Rationale*: a {{SqlOperator}} has a lifetime that spans many statements; but
> a type factory is only for one statement, and each type belongs to that
> factory. We want to share {{SqlOperator}} instances across connections,
> therefore we need to create them before there is a type factory.
> Typically, a method that returns a type should have a type factory argument
> with which to create it.
> The current situation is technical debt. There are a couple of pieces of code
> tagged with this case number; see the fix to CALCITE-2072.
> In particular:
> * Remove method {{List SqlOperator.getParamTypes()}};
> * Remove {{RelDataTypeFactory}} argument from {{SqlUserDefinedAggFunction}}
> constructor, and remove its {{typeFactory}} field.
> We will add {{interface SqlOperandMetadata extends SqlOperatorTypeChecker}},
> which has new methods {{List> paramTypes(RelDataTypeFactory)}}
> and {{List paramNames()}}.
> This interface will typically be implemented only for user-defined functions.
> Unlike SQL built-in functions, UDFs have a fixed set of parameters (although
> some of them may be optional), and the parameters have names.
> In {{interface SqlOperandTypeChecker}}, add method {{boolean
> isFixedParameters()}}. Will typically return true for UDFs, false for
> built-in functions. Returns false for table window functions (e.g. {{HOP}}),
> even though these have named parameters (which tends to make them look a bit
> like UDFs).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4253) RelOptUtil#findAllTables should probably use RelMetadataQuery#getTableReferences
[
https://issues.apache.org/jira/browse/CALCITE-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195654#comment-17195654
]
Jesus Camacho Rodriguez commented on CALCITE-4253:
--
Thanks for pinging me [~julianhyde]. It's been some time but I believe
{{RelOptUtil.findAllTables}} relying on {{RelMetadataQuery}} was simply to have
a single code path, which may not have been the right choice based on your
comment above. As for not using {{getTableReferences}}, maybe it was introduced
after CALCITE-1682? Automatic caching (since the rule may make the same call
repeatedly) and extensibility were the main factors to rely on the metadata
providers in the MV rewriting rules indeed.
> RelOptUtil#findAllTables should probably use
> RelMetadataQuery#getTableReferences
>
>
> Key: CALCITE-4253
> URL: https://issues.apache.org/jira/browse/CALCITE-4253
> Project: Calcite
> Issue Type: Sub-task
> Components: core
>Affects Versions: 1.25.0
>Reporter: Vladimir Sitnikov
>Priority: Minor
>
> It looks like both methods do exactly the same thing, so it would probably
> make sense to divert {{findAllTables}} to
> {{RelMetadataQuery#getTableReferences}}
> If methods are different, it would make sense to add the relevant
> documentation and cross-references.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (CALCITE-4256) RexSimplify should not simplify P AND P to P, if it contains a call to RAND or RAND_INTEGER
Thomas Rebele created CALCITE-4256:
--
Summary: RexSimplify should not simplify P AND P to P, if it
contains a call to RAND or RAND_INTEGER
Key: CALCITE-4256
URL: https://issues.apache.org/jira/browse/CALCITE-4256
Project: Calcite
Issue Type: Bug
Reporter: Thomas Rebele
Example: RAND_INTEGER() = 1 AND RAND_INTEGER() = 1 is false with a higher
probability than RAND_INTEGER() = 1
Here a test case for RexProgramTest:
{code}
@Test void testSimplifyRandomAnd() {
checkSimplifyUnchanged(
and(
eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
literal(1)),
eq(rexBuilder.makeCall(SqlStdOperatorTable.RAND_INTEGER),
literal(1))
));
}
{code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map
[
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195637#comment-17195637
]
Vladimir Sitnikov edited comment on CALCITE-4254 at 9/14/20, 5:32 PM:
--
Julian, what do you think ofrenaming {{makeImmutable}} attribute to
{{mutable}}, so the use becomes {{makeMutable=true}} rather than
{{immutable=false}}?
---
One more idea might be to use Java 1.8 type annotations, so the declaration
might be like
{code:java}
@ImmutableBean.Property
@Mutable List getNumbers();
{code}
A nice consequence would be the same design might work for cases like
{code:java}Map>{code}, and, more important,
{{@Mutable}} annotations would be visible in the IDE, so users see if the list
is mutable or not.
was (Author: vladimirsitnikov):
Julian, what do you think ofrenaming {{makeImmutable}} attribute to
{{mutable}}, so the use becomes {{makeMutable=true}} rather than
{{immutable=false}}?
---
One more idea might be to use Java 1.8 type annotations, so the declaration
might be like
{code:java}
@ImmutableBean.Property
@Mutable List getNumbers();
{code}
A nice consequence would be the same design might work for cases like
{code:java}Map>{code}, and, more important,
{{@Mutable}} annotations would be visible in the IDE, so users see that the
list is mutable or not.
> ImmutableBeans should make an immutable copy of property values of type List,
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}},
> {{Set}} or {{Map}} when they are provided via a setter method. For example,
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you
> declare the property:
> {code:java}
> interface MyBean {
> @ImmutableBean.Property(makeImmutable = false)
> List getNumbers();
> MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very
> few existing beans that were assuming that the value that was put in would be
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}),
> but for the vast majority, immutability is the desired behavior.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map
[
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195637#comment-17195637
]
Vladimir Sitnikov edited comment on CALCITE-4254 at 9/14/20, 5:27 PM:
--
Julian, what do you think ofrenaming {{makeImmutable}} attribute to
{{mutable}}, so the use becomes {{makeMutable=true}} rather than
{{immutable=false}}?
---
One more idea might be to use Java 1.8 type annotations, so the declaration
might be like
{code:java}
@ImmutableBean.Property
@Mutable List getNumbers();
{code}
A nice consequence would be the same design might work for cases like
{code:java}Map>{code}, and, more important,
{{@Mutable}} annotations would be visible in the IDE, so users see that the
list is mutable or not.
was (Author: vladimirsitnikov):
Julian, what do you think ofrenaming {{makeImmutable}} attribute to
{{mutable}}, so the use becomes {{makeMutable=true}} rather than
{{immutable=false}}?
Just in case, another idea might be Java 1.8 type annotations, so the
declaration might be like
{code:java}
@ImmutableBean.Property
@Mutable List getNumbers();
{code}
A nice consequence would be the same design might work for cases like
{code:java}Map>{code}, and, more important,
{{@Mutable}} annotations would be visible in the IDE, so users see that the
list is mutable or not.
> ImmutableBeans should make an immutable copy of property values of type List,
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}},
> {{Set}} or {{Map}} when they are provided via a setter method. For example,
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you
> declare the property:
> {code:java}
> interface MyBean {
> @ImmutableBean.Property(makeImmutable = false)
> List getNumbers();
> MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very
> few existing beans that were assuming that the value that was put in would be
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}),
> but for the vast majority, immutability is the desired behavior.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map
[
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195637#comment-17195637
]
Vladimir Sitnikov commented on CALCITE-4254:
Julian, what do you think ofrenaming {{makeImmutable}} attribute to
{{mutable}}, so the use becomes {{makeMutable=true}} rather than
{{immutable=false}}?
Just in case, another idea might be Java 1.8 type annotations, so the
declaration might be like
{code:java}
@ImmutableBean.Property
@Mutable List getNumbers();
{code}
A nice consequence would be the same design might work for cases like
{code:java}Map>{code}, and, more important,
{{@Mutable}} annotations would be visible in the IDE, so users see that the
list is mutable or not.
> ImmutableBeans should make an immutable copy of property values of type List,
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}},
> {{Set}} or {{Map}} when they are provided via a setter method. For example,
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you
> declare the property:
> {code:java}
> interface MyBean {
> @ImmutableBean.Property(makeImmutable = false)
> List getNumbers();
> MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very
> few existing beans that were assuming that the value that was put in would be
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}),
> but for the vast majority, immutability is the desired behavior.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4249) Assertion error for NOT operator in join condition in Rel2SqlConverter
[
https://issues.apache.org/jira/browse/CALCITE-4249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195630#comment-17195630
]
Julian Hyde commented on CALCITE-4249:
--
I don't think it's a duplicate. There is a proposed fix in
https://github.com/apache/calcite/pull/2147.
> Assertion error for NOT operator in join condition in Rel2SqlConverter
> --
>
> Key: CALCITE-4249
> URL: https://issues.apache.org/jira/browse/CALCITE-4249
> Project: Calcite
> Issue Type: Bug
>Reporter: Steven Talbot
>Assignee: Julian Hyde
>Priority: Major
>
> The following test shows the bug if you put it in RelToSqlConverter
> {code:java}
> @Test void testJoinWithNotLikeConditionRel2Sql() {
> final Function relFn = b -> b
> .scan("EMP")
> .scan("DEPT")
> .join(JoinRelType.LEFT,
> b.and(
> b.call(SqlStdOperatorTable.EQUALS,
> b.field(2, 0, "DEPTNO"),
> b.field(2, 1, "DEPTNO")),
> b.call(SqlStdOperatorTable.NOT,
> b.call(SqlStdOperatorTable.LIKE,
> b.field(2, 1, "DNAME"),
> b.literal("ACCOUNTING_FOO"))
> )
> ))
> .build();
> final String expectedSql = "SELECT *\n"
> + "FROM \"scott\".\"EMP\"\n"
> + "LEFT JOIN \"scott\".\"DEPT\" "
> + "ON \"EMP\".\"DEPTNO\" = \"DEPT\".\"DEPTNO\" "
> + "AND \"DEPT\".\"DNAME\" NOT LIKE 'ACCOUNTING'";
> relFn(relFn).ok(expectedSql);
> }
> {code}
> It blows up with the following stacktrace top:
> {noformat}
> java.lang.AssertionError: NOT(LIKE($9, 'ACCOUNTING_FOO'))
> at
> org.apache.calcite.rel.rel2sql.SqlImplementor.convertConditionToSqlNode(SqlImplementor.java:350)
> at
> org.apache.calcite.rel.rel2sql.SqlImplementor.convertConditionToSqlNode(SqlImplementor.java:286)
> at
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visit(RelToSqlConverter.java:213)
> at org.apache.calcite.util.ReflectUtil$2.invoke(ReflectUtil.java:524)
> at
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.dispatch(RelToSqlConverter.java:131)
> at
> org.apache.calcite.rel.rel2sql.RelToSqlConverter.visitInput(RelToSqlConverter.java:139){noformat}
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4253) RelOptUtil#findAllTables should probably use RelMetadataQuery#getTableReferences
[
https://issues.apache.org/jira/browse/CALCITE-4253?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195626#comment-17195626
]
Julian Hyde commented on CALCITE-4253:
--
Yeah, it's not great. If you read the comments on
{{RelOptCluster.getMetadataQuery()}} you'll see it's discouraged.
[~jcamachorodriguez], Do you recall why, in CALCITE-1682, you moved the code
into {{RelMetadataQuery}} (creating {{RelMetadataQuery.getNodeTypes}} in the
process). Maybe you needed caching or extensibility?
> RelOptUtil#findAllTables should probably use
> RelMetadataQuery#getTableReferences
>
>
> Key: CALCITE-4253
> URL: https://issues.apache.org/jira/browse/CALCITE-4253
> Project: Calcite
> Issue Type: Sub-task
> Components: core
>Affects Versions: 1.25.0
>Reporter: Vladimir Sitnikov
>Priority: Minor
>
> It looks like both methods do exactly the same thing, so it would probably
> make sense to divert {{findAllTables}} to
> {{RelMetadataQuery#getTableReferences}}
> If methods are different, it would make sense to add the relevant
> documentation and cross-references.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map
[
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195608#comment-17195608
]
Vladimir Sitnikov commented on CALCITE-4254:
Oh, I just re-read the issue description, and now I see it suggests "immutable
by default, and add escape hatch for a mutable case". That is awesome for me.
> ImmutableBeans should make an immutable copy of property values of type List,
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}},
> {{Set}} or {{Map}} when they are provided via a setter method. For example,
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you
> declare the property:
> {code:java}
> interface MyBean {
> @ImmutableBean.Property(makeImmutable = false)
> List getNumbers();
> MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very
> few existing beans that were assuming that the value that was put in would be
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}),
> but for the vast majority, immutability is the desired behavior.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map
[
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195607#comment-17195607
]
Vladimir Sitnikov commented on CALCITE-4254:
Well, there's one more option: use immutable by default, and provide a way to
unlock mutable when it is needed.
> ImmutableBeans should make an immutable copy of property values of type List,
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}},
> {{Set}} or {{Map}} when they are provided via a setter method. For example,
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you
> declare the property:
> {code:java}
> interface MyBean {
> @ImmutableBean.Property(makeImmutable = false)
> List getNumbers();
> MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very
> few existing beans that were assuming that the value that was put in would be
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}),
> but for the vast majority, immutability is the desired behavior.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4254) ImmutableBeans should make an immutable copy of property values of type List, Set, or Map
[
https://issues.apache.org/jira/browse/CALCITE-4254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195601#comment-17195601
]
Julian Hyde commented on CALCITE-4254:
--
[~vlsi], Ideally we'd allow both ways. In your scheme, the wither method would
have type {{List}} and the getter method would have type {{ImmutableList}}. In
my scheme, the wither and getter would both have type {{List}}.
My scheme would be preferred by people who prefer to keep Guava types out of
interfaces. Plus, the implementation currently does not allow the property to
have different types in wither and getter.
> ImmutableBeans should make an immutable copy of property values of type List,
> Set, or Map
> -
>
> Key: CALCITE-4254
> URL: https://issues.apache.org/jira/browse/CALCITE-4254
> Project: Calcite
> Issue Type: Bug
>Reporter: Julian Hyde
>Assignee: Julian Hyde
>Priority: Major
>
> {{ImmutableBeans}} should make an immutable copy of values of type {{List}},
> {{Set}} or {{Map}} when they are provided via a setter method. For example,
> this is how it should work:
> {code:java}
> MyBean bean = ImmutableBeans.create(MyBean.class);
> List evens = Arrays.asList(2, 4, 6);
> bean = bean.withNumbers(evens);
> assertThat(bean.getNumbers(), equalTo(evens));
> assertThat(bean.getNumbers(), isInstanceOf(ImmutableList.class));
> {code}
> Note that {{evens}} has been converted to an {{ImmutableList}} with the same
> contents.
> You can control this behavior by setting {{makeImmutable = false}} when you
> declare the property:
> {code:java}
> interface MyBean {
> @ImmutableBean.Property(makeImmutable = false)
> List getNumbers();
> MyBean withNumbers(List numbers);
> }
> {code}
> By default, {{makeImmutable}} is true. This will change behavior of a very
> few existing beans that were assuming that the value that was put in would be
> the value that came out (see a couple of changes in {{VolcanoPlannerTest}}),
> but for the vast majority, immutability is the desired behavior.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4251) Overload RelMetadataQuery#getColumnOrigin method
[
https://issues.apache.org/jira/browse/CALCITE-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195593#comment-17195593
]
Julian Hyde commented on CALCITE-4251:
--
I see. With today's implementation, I don't it's possible to overload. However,
some providers (e.g. {{interface BuiltInMetadata.Size}} have multiple methods).
Please take a look at {{interface BuiltInMetadata.ColumnOrigin}} and say how
you propose to change it.
> Overload RelMetadataQuery#getColumnOrigin method
>
>
> Key: CALCITE-4251
> URL: https://issues.apache.org/jira/browse/CALCITE-4251
> Project: Calcite
> Issue Type: Wish
>Reporter: xzh_dz
>Priority: Major
>
> A case:
>
> {code:java}
> //
> select col1, col2 , sum(count_col4) as sum_col4
> from (
> select col1, col2, col3, count(col4) as count_col4
> from tbl
> group by col1, col2, col3
> )
> group by col1, col2
> {code}
> When i try to get the `sum_col4` origin column field,it will return null and
> it is derived. we always get the origin column although it is derived. We
> should overload this method.
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4251) Overload RelMetadataQuery#getColumnOrigin method
[
https://issues.apache.org/jira/browse/CALCITE-4251?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17195434#comment-17195434
]
xzh_dz commented on CALCITE-4251:
-
Should we indicate that a parameter `derived` for table columns may be derived?
`derived` is a Boolean value, the default value can be set to false.
> Overload RelMetadataQuery#getColumnOrigin method
>
>
> Key: CALCITE-4251
> URL: https://issues.apache.org/jira/browse/CALCITE-4251
> Project: Calcite
> Issue Type: Wish
>Reporter: xzh_dz
>Priority: Major
>
> A case:
>
> {code:java}
> //
> select col1, col2 , sum(count_col4) as sum_col4
> from (
> select col1, col2, col3, count(col4) as count_col4
> from tbl
> group by col1, col2, col3
> )
> group by col1, col2
> {code}
> When i try to get the `sum_col4` origin column field,it will return null and
> it is derived. we always get the origin column although it is derived. We
> should overload this method.
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
