[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-12 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-585166370
 
 
   I've created https://issues.apache.org/jira/browse/CALCITE-3786 for Digest.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-11 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-584851082
 
 
   I wonder what @julianhyde  and @zabetak think of 
https://github.com/apache/calcite/pull/1740#issuecomment-584848797


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-11 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-584850203
 
 
   > We don't build strings over and over
   
   Every time we create a new RelType we build stings.
   Every time we create RelNode we build strings.
   Every time we create RexNode we build strings.
   
   This is relevant: https://issues.apache.org/jira/browse/CALCITE-3784


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-11 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-584848797
 
 
   > second string is the new digest
   
   The use of stings for digests is unfortunate. It takes time to build a 
string, however in practice we don't need that as a single object.
   
   What I mean is something behind the lines of
   
   ```java
   class RelDataType {
  Digest digest;
  Digest getDigestsSansFieldNames() {
  Digest digest = this.digset;
  if (digest != null) return digest;
  this.digest = digest = ...;
  return digest;
  }
   }
   
   class Digest {
  final int hashCode;
  final Object[] contents;
   }
   ```
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-11 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-584799605
 
 
   @xndai , I would rather add a method to `RelDataType` that computes the 
digest without taking column names to the consideration.
   
   Note: we add an opaque `Digest` interface (or even `Object`), so it does not 
tie to `String`.
   It looks like we have similar issues with `RexNode` digest, and `RelNode` 
digest. There we build strings again and again, and it might help if we would 
be able to skip building strings.
   
   I don't insist that adding `Digestable` interface is the only way to go 
here, however, I think it is a nice opportunity, and it might be helpful for 
other digests as well.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-11 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-584754624
 
 
   Just in case. Removal of the field names from 
`RelRecordType::generateTypeString` produces extreme amount of test failures: 
https://github.com/apache/calcite/pull/1797
   
   So it does not look like we can make such a change.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-11 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-584548560
 
 
   @xndai , I just thought removal of column name from 
`RelRecordType::generateTypeString` might have non-trivial side effects for 
Calcite consumers.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [calcite] vlsi commented on issue #1740: [CALCITE-3713] Remove column names from Project#digest

2020-02-07 Thread GitBox
vlsi commented on issue #1740: [CALCITE-3713] Remove column names from 
Project#digest
URL: https://github.com/apache/calcite/pull/1740#issuecomment-583565364
 
 
   @hsyuan , do you have async-profiler or JFR snapshots?


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:
us...@infra.apache.org


With regards,
Apache Git Services