[jira] [Created] (CALCITE-6395) Significant precision loss when representing REAL literals

2024-05-01 Thread Mihai Budiu (Jira)
Mihai Budiu created CALCITE-6395:


 Summary: Significant precision loss when representing REAL literals
 Key: CALCITE-6395
 URL: https://issues.apache.org/jira/browse/CALCITE-6395
 Project: Calcite
  Issue Type: Bug
  Components: core
Affects Versions: 1.36.0
Reporter: Mihai Budiu


Consider this test that could be a SqlOperatorTest:

{code:java}
f.checkScalar("CAST(CAST('36854775807.0' AS REAL) AS BIGINT)",
"36854775808", "BIGINT NOT NULL");
{code}

The produced result is actually very far:

Expected: is "36854775808"
 but: was "36854779904"

This big error comes from two reasons:
- Calcite uses BigDecimal values to represent floating point values, see 
[CALCITE-2067]
- When converting a Float value to a BigDecimal in RexBuilder.clean(), the 
following sequence is used: 

{code:java}
new BigDecimal(((Number) o).doubleValue(), MathContext.DECIMAL32)
{code}

Using a DECIMAL32 math context leads to the precision loss. Just because a 
Float uses 32 bits does not mean that the decimal should also use 32 bits.

The real fix is in the PR for [CALCITE-2067], but that hasn't been reviewed for 
a long time, so I will submit a fix for the clean() method..




--
This message was sent by Atlassian Jira
(v8.20.10#820010)


Re: [VOTE] Release Apache Calcite 1.37.0 (release candidate 4)

2024-05-01 Thread Ruben Q L
Thanks Sergey for preparing this new RC.

- Release notes: ok (minor comment left in the PR)
- Checksum: ok
- Signature: ok
- Diff source release and git repository: ok
- Build + tests (from both RC commit and source artifacts): ok (also
confirmed in a Windows env that the ArrowAdapter test issue CALCITE-6390 is
fixed)
- Calcite-based application test suite: ok

+0 (binding)
I would prefer to have the ASM issue fixed or at least workarounded in
1.37, but I understand this is not a new problem and strictly speaking it
is not a Calcite source issue; let's try to find a clean solution for the
next release 1.38 via CALCITE-6393.



On Wed, May 1, 2024 at 12:16 AM Julian Hyde  wrote:

> It seems that Sergey has logged
> https://issues.apache.org/jira/browse/CALCITE-6393 for this. Please add
> further discussion to that case rather than to this vote thread.
>
>
>
> > On Apr 30, 2024, at 12:44 AM, Ruben Q L  wrote:
> >
> > Thanks Guillaume for checking this!
> > When I looked at this issue on 1.36, I had the impression that assertions
> > might have a role to play in the error, but I could not confirm this
> > hypothesis.
> > IMO this smells like a JDK bug (or at least it looks like other resolved
> > issues); note that it seems we had some similar problems in the past with
> > older Java8 versions [1].
> >
> > Independently of the root cause, would it unblock the release process if
> we
> > just remove the problematic asserts, or if we substitute them with an
> > equivalent `if (...) throw new IllegalStateException("...");` ?
> >
> > Best,
> > Ruben
> >
> > [1]
> >
> https://github.com/apache/calcite/blob/1506857f404037b63dfd8a11880393b767bd1544/build.gradle.kts#L98
> >
> >
> >
> > On Mon, Apr 29, 2024 at 11:26 PM Sergey Nuyanzin 
> > wrote:
> >
> >> Hi Guilluame,
> >>
> >> I played a bit more and I realised that if from commit above I just
> remove
> >> one line with assert (assert map != null)
> >> then ArrayIndexOutOfBoundsException disappears
> >>
> >> same for current main branch, if I remove all lines from SqlFunctions
> with
> >> assert (now there are 3 such lines) then
> >> ArrayIndexOutOfBoundsException disappears
> >>
> >> Thus,  it does not look like a Calcite issue, more like a problem on ASM
> >> side
> >> please correct me if I'm wrong
> >>
> >> On Mon, Apr 29, 2024 at 10:33 PM Sergey Nuyanzin 
> >> wrote:
> >>
> >>> i follow the procedure described here
> >>> https://calcite.apache.org/docs/howto.html#making-a-release-candidate
> >>> started
> >>> btw I played a bit with git bisect and it shows that the issue
> >>>
> >>>java.lang.ArrayIndexOutOfBoundsException: Index 65536 out of bounds
> >> for length 297at
> >> org.objectweb.asm.ClassReader.readLabel(ClassReader.java:2695)at
> >> org.objectweb.asm.ClassReader.createLabel(ClassReader.java:2711)
> >>>
> >>> started appearing after this commit
> >>>
> >>>
> >>
> https://github.com/apache/calcite/commit/bcf6bd8577b25c563b1c597c70704594a18ca1a3
> >>>
> >>> On Mon, Apr 29, 2024 at 10:01 PM Guillaume Masse
> >>>  wrote:
> >>>
>  Hi Sergey,
> 
>  thanks for trying that update
> 
>  I confirm I have the same issue with your release at
> 
> 
> >>
> https://repository.apache.org/content/repositories/orgapachecalcite-1231/org/apache/calcite/
> 
> 
> 
> >>
> https://github.com/MasseGuillaume/asm-remapper-bug/commit/852e4cd246d278db8acf5e997a54619bc4f85fc7
> 
>  This rules out one of my hypothesis on the java build version.
> 
>  Can you point me to the release procedure you are using? I saw
>  https://calcite.apache.org/develop/#contributing is there more
> precise
>  steps?
> 
> 
> 
>  On Mon, Apr 29, 2024 at 3:27 PM Sergey Nuyanzin 
>  wrote:
> 
> > I repeated same procedure with jdk1.8u412
> > here you can find the jars
> >
> >
> 
> >>
> https://repository.apache.org/content/repositories/orgapachecalcite-1231/org/apache/calcite/
> > it looks like asm-remapper gives the same result...
> >
> > On Mon, Apr 29, 2024 at 8:41 PM Guillaume Masse
> >  wrote:
> >
> >> If you take a look at
> >>
> >>
> >>
> >
> 
> >>
> https://www.openlogic.com/openjdk-downloads?field_java_parent_version_target_id=416_operating_system_target_id=426_architecture_target_id=391_java_package_target_id=All
> >>
> >> The most recent release is 8u412-b08
> >>
> >> If it follows more or less the openjdk schedule:
> >>
> >> https://wiki.openjdk.org/display/jdk8u/Main
> >>
> >> *Most recent and past release details:*
> >>
> >>   - 8u412-b08 (GA), April 16th 2024 [Release <
> > https://bit.ly/openjdk8u412
> >>> ]
> >>   [Tag  >>> ]
>  [
> >>   Binaries
> >>   <
> >>
> 
> https://github.com/adoptium/temurin8-binaries/releases/tag/jdk8u412-b08
> >>>
> >>   ]
> >>   - 8u402-b06 (GA), 

Re: Is it posible to change synthetic input names ($0, $1, etc) in explain plans?

2024-05-01 Thread Gonzalo Ortiz Jaureguizar
Thanks for your fast response. I've created
https://issues.apache.org/jira/browse/CALCITE-6394 to discuss about that.


[jira] [Created] (CALCITE-6394) Be able to use input names instead of $i in explain

2024-05-01 Thread Gonzalo Ortiz (Jira)
Gonzalo Ortiz created CALCITE-6394:
--

 Summary: Be able to use input names instead of $i in explain
 Key: CALCITE-6394
 URL: https://issues.apache.org/jira/browse/CALCITE-6394
 Project: Calcite
  Issue Type: Improvement
Reporter: Gonzalo Ortiz


As Apache Pinot committer, one of the request I've found from our users is to 
make explains easier to read. The main complain here is that it requires some 
training to be able to read them, specially because in explain names of the 
inputs is index based ($0, $1, etc) but the rows itself are indirectly defined 
by the each input. A person with enough experience may understand that the rows 
generated by a join is formed by appending the input of the left hand side with 
the input of the right hand side, but in complex queries it takes time to get 
use to that and to picture these rows in your mind.

These complains are also based by the fact that other databases have solved 
this issue long ago. For example, in Postgres, given a table `a` and `two_cols` 
such as:

{{postgres=# \d a}}
{{Table "public.a"}}
{{Column |  Type   | Collation | Nullable | Default  }}
{{+-+---+--+-}}
{{a  | integer |   |  |  }}

{{postgres=# \d two_cols}}
{{ Table "public.two_cols"}}
{{Column |  Type   | Collation | Nullable | Default  }}
{{+-+---+--+-}}
{{a  | integer |   |  |  }}
{{b  | integer |   |  |}}

A join explain shows:

{{postgres=# explain select * from a join two_cols on a.a = two_cols.a;}}
{{  QUERY PLAN    }}
{{}}
{{Merge Join  (cost=338.29..781.81 rows=28815 width=12)}}
{{  Merge Cond: ({*}two_cols.a = a.a{*})}}
{{  ->  Sort  (cost=158.51..164.16 rows=2260 width=8)}}
{{Sort Key: *two_cols.a*}}
{{->  Seq Scan on two_cols  (cost=0.00..32.60 rows=2260 width=8)}}
{{  ->  Sort  (cost=179.78..186.16 rows=2550 width=4)}}
{{Sort Key: *a.a*}}
{{->  Seq Scan on a  (cost=0.00..35.50 rows=2550 width=4)}}
{{(8 rows)}}

 

In Postgres the explain even conserves the name of the aliases if used.

To be clear, I don't know if it is possible to do this right now. I didn't find 
a way myself and after asking in the dev mail least I was invited to write a 
Jira issue to discuss about the specific requirements.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)