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

workload: check that TPCH workload returns expected results #34891

Merged
merged 1 commit into from
Jun 28, 2019

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Feb 14, 2019

Now, by default, whenever TPCH workload is being run on scale factor of 1,
the returned output is checked against the expected results (which have been
roughly confirmed with PostgreSQL). This behavior can be disabled
with a flag --disable-checks=true to the workload command.

Release note: None

@yuzefovich yuzefovich added the do-not-merge bors won't merge a PR with this label. label Feb 14, 2019
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich
Copy link
Member Author

Some concerns are:

  1. I modified date output from 1994-04-07 00:00:00+00:00 (returned by our cli) to 1994-04-07 00:00:00 +0000 +0000 (returned by the workload)
  2. an error on query 11
  3. 6202 represented by ASCII codes by the workload ([54 50 48 50])

@awoods187 awoods187 mentioned this pull request Mar 5, 2019
8 tasks
@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label Jun 19, 2019
Copy link
Member

@jordanlewis jordanlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: let's get this merged, although we won't currently pass.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @yuzefovich)


pkg/workload/tpch/tpch.go, line 56 at r1 (raw file):

}

// TODO(yuzefovich): this seems to not be used anywhere, should we delete it?

init functions get automatically run by Go.

@yuzefovich yuzefovich force-pushed the tpch_expected branch 3 times, most recently from 75afd29 to 989f97a Compare June 27, 2019 20:46
@yuzefovich yuzefovich changed the title [WIP] workload: check that TPCH workload returns expected results workload: check that TPCH workload returns expected results Jun 27, 2019
Now, by default, whenever TPCH workload is being run, the returned
output is checked against the expected results (which have been
roughly confirmed with PostgreSQL). This behavior can be disabled
with a flag `--disable-checks=true` to the workload command.

Release note: None
Copy link
Member Author

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query 11 has since been fixed, but it returns 1048 rows, so I added a check only for the number of rows returned for it (same as query 16).

Query 12 is still skipped (I'm not sure what the problem is, running query through cli gives the expected result, but via a workload it for some reason returns ASCII encoding of the output Error: wrong result on query 12 in row 0 in column 1: got "[54 50 48 50]", expected "6202").

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @jordanlewis)


pkg/workload/tpch/tpch.go, line 56 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

init functions get automatically run by Go.

Yeah, I didn't know about this at the time, thanks!

@yuzefovich yuzefovich removed the do-not-merge bors won't merge a PR with this label. label Jun 27, 2019
@yuzefovich
Copy link
Member Author

TFTR!

bors r+

craig bot pushed a commit that referenced this pull request Jun 28, 2019
34891: workload: check that TPCH workload returns expected results r=yuzefovich a=yuzefovich

Now, by default, whenever TPCH workload is being run on scale factor of 1,
the returned output is checked against the expected results (which have been
roughly confirmed with PostgreSQL). This behavior can be disabled
with a flag `--disable-checks=true` to the workload command.

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
@craig
Copy link
Contributor

craig bot commented Jun 28, 2019

Build succeeded

@craig craig bot merged commit d0511fb into cockroachdb:master Jun 28, 2019
@yuzefovich yuzefovich deleted the tpch_expected branch June 28, 2019 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants