You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/12/04 08:40:52 UTC
[GitHub] [calcite] Aaaaaaron opened a new pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Aaaaaaron opened a new pull request #2291:
URL: https://github.com/apache/calcite/pull/2291
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] MalteBellmann commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
MalteBellmann commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-738664101
Hi,
wouldn't it be easier to read and more general to implement this short-circuit in RelTraitSet#satisfies, as it is explained in the JavaDoc that "each trait set satisfies itself"?
With this change:
public boolean satisfies(RelTraitSet that) {
if (this == that){
return true;
}
...now the normal check...
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-773065068
LGTM
if there is no objections, I will merge this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liyafan82 commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-740444380
LGTM. This is clearly a safe improvment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia merged pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
amaliujia merged pull request #2291:
URL: https://github.com/apache/calcite/pull/2291
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] MalteBellmann edited a comment on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
MalteBellmann edited a comment on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-738664101
Hi,
wouldn't it be easier to read and more general to implement this short-circuit in RelTraitSet#satisfies, as it is explained in the JavaDoc that "each trait set satisfies itself"?
With this change:
```
public boolean satisfies(RelTraitSet that) {
if (this == that){
return true;
}
// now the normal check...
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron edited a comment on pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron edited a comment on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-772188502
Hi @amaliujia, could you help please review this PR? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-772188502
Hi @amaliujia, would you please review this PR? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-740491994
@Aaaaaaron , please remove `to improve the performance of RelSubset#getRelList` from JIRA and from the commit message.
The commit has virtually nothing to do with `RelSubset#getRelList`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-738658931
Does anyone know about this?
* What went wrong:
Execution failed for task ':rat'.
> A failure occurred while executing org.nosphere.apache.rat.RatWork
> Apache Rat audit failure - 1 unapproved license
See file:///Users/runner/work/calcite/calcite/build/reports/rat/index.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-773065068
LGTM
if there is no objections, I will merge this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] amaliujia merged pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
amaliujia merged pull request #2291:
URL: https://github.com/apache/calcite/pull/2291
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron edited a comment on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron edited a comment on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-738658931
Does anyone know about this?
```
* What went wrong:
Execution failed for task ':rat'.
> A failure occurred while executing org.nosphere.apache.rat.RatWork
> Apache Rat audit failure - 1 unapproved license
See file:///Users/runner/work/calcite/calcite/build/reports/rat/index.html
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-738673472
> Hi,
> wouldn't it be easier to read and more general to implement this short-circuit in RelTraitSet#satisfies, as it is explained in the JavaDoc that "each trait set satisfies itself"?
> With this change:
>
> ```
> public boolean satisfies(RelTraitSet that) {
> if (this == that){
> return true;
> }
>
> // now the normal check...
> }
> ```
Yes, it's better, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2291: [CALCITE-4426] Short-circuit evaluating trait set equals to improve the performance of RelSubset#getRelList (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron commented on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-740600153
> @Aaaaaaron , please remove `to improve the performance of RelSubset#getRelList` from JIRA and from the commit message.
> The commit has virtually nothing to do with `RelSubset#getRelList`
Sure, in the first version, the change is in getRelList and after I update the code, I forgot to change the message, thanks @vlsi !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] Aaaaaaron removed a comment on pull request #2291: [CALCITE-4426] Short-circuit evaluating when comparing two "RelTraitSet"s (Jiatao Tao)
Posted by GitBox <gi...@apache.org>.
Aaaaaaron removed a comment on pull request #2291:
URL: https://github.com/apache/calcite/pull/2291#issuecomment-738658931
Does anyone know about this?
```
* What went wrong:
Execution failed for task ':rat'.
> A failure occurred while executing org.nosphere.apache.rat.RatWork
> Apache Rat audit failure - 1 unapproved license
See file:///Users/runner/work/calcite/calcite/build/reports/rat/index.html
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org