Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong answer for TPC-H Q17 #6413

Closed
monetdb-team opened this issue Nov 30, 2020 · 0 comments
Closed

Wrong answer for TPC-H Q17 #6413

monetdb-team opened this issue Nov 30, 2020 · 0 comments

Comments

@monetdb-team
Copy link

@monetdb-team monetdb-team commented Nov 30, 2020

Date: 2017-10-06 15:05:21 +0200
From: @drstmane
To: SQL devs <>
Version: 11.27.5 (Jul2017-SP1)
CC: @njnes

Last updated: 2017-10-26 14:01:43 +0200

Comment 25678

Date: 2017-10-06 15:05:21 +0200
From: @drstmane

Somewhere between changeset 702993d1f2e0 and changeset 36755134ba9d --- on the Dec2016 branch --- a code change caused TPC-H Q17 (on default SF-1 dataset with default query parameters) to produce a wrong result.

While the "official" TPC-H results is

348406.02

MonetDB used to produce the ("officially"?) "acceptable" result

348406.054

up until (and including) changeset 702993d1f2e0
At least since changeset 36755134ba9d --- I haven't tested intermediate changesets, yet --- MonetDB now produces the wrong result

3270416.820

which is off by more than 800% ...

Comment 25679

Date: 2017-10-06 16:22:51 +0200
From: @drstmane

The "culprit" appears to be changeset b72b05357cdd .

Comment 25680

Date: 2017-10-06 16:39:31 +0200
From: @drstmane

Created attachment 569
Differences in PLAN, EXPLAIN, TRACE for TPCH Q17 between changesets 99aabb9cfcc6 & b72b05357cdd

The attached HTML file shows the differences in PLAN, EXPLAIN, and TRACE for TPCH Q17 on SF-1 (single-threaded) between changesets 99aabb9cfcc6 ("last good") and b72b05357cdd ("first bad").

Attached file: TPCH-SF1-Q17-99aabb9cfcc6-vs-b72b05357cdd.html (text/html, 67586 bytes)
Description: Differences in PLAN, EXPLAIN, TRACE for TPCH Q17 between changesets 99aabb9cfcc6 & b72b05357cdd

Comment 25690

Date: 2017-10-11 16:46:59 +0200
From: @drstmane

Created attachment 572
Differences in PLAN, EXPLAIN, TRACE, RESULT for a revised version of TPCH Q17 between changesets 99aabb9cfcc6 & b72b05357cdd (edit details)

The differences in the EXPLAIN of Q17 between changesets 99aabb9cfcc6 & b72b05357cdd suggest that in particular the last algebra.projectionpath() differs as follows:

99aabb9cfcc6 (GOOD):
X_445:bat[:lng] := algebra.projectionpath(C_433,r1_86,r1_72,X_372,C_327,X_435);

b72b05357cdd (BAD):
X_444:bat[:lng] := algebra.projectionpath(C_433,r1_86, X_386 ,C_327,X_435);

resulting in picking the wrong lineitem tuples for the final result.

The attached differences in PLAN, EXPLAN, TRACE, and RESULT between changesets 99aabb9cfcc6 & b72b05357cdd for a revised version of Q17 --- that dumps the relevant final intermediate result instead of performing the global aggregation --- seems to confirm this.

Attached file: TPCH-SF1-Q17x-99aabb9cfcc6-vs-b72b05357cdd.html (text/html, 543137 bytes)
Description: Differences in PLAN, EXPLAIN, TRACE, RESULT for a revised version of TPCH Q17 between changesets 99aabb9cfcc6 & b72b05357cdd (edit details)

Comment 25691

Date: 2017-10-11 21:30:31 +0200
From: @njnes

it seems the outer and inner 'lineitem' table are swapped. When I change the sub
query using aliases (lineitem l1 and before its columns l1.), it works again.

Comment 25692

Date: 2017-10-11 22:35:33 +0200
From: @njnes

fixed, correctly mark all internal conflicting columns as used/rename all of them. This solve q17, ie it uses the proper extendedprice

Comment 25693

Date: 2017-10-11 22:37:35 +0200
From: MonetDB Mercurial Repository <>

Changeset 055d1f50bb0f made by Niels Nes niels@cwi.nl in the MonetDB repo, refers to this bug.

For complete details, see https//devmonetdborg/hg/MonetDB?cmd=changeset;node=055d1f50bb0f

Changeset description:

fixed bug #6413, ie correct output of tpch sf1 q17. A column from
the subquery was used in the outer query. This is fixed in the
apply/rename optimizer (which correctly marks all internal columns
as such and renames them).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant