Skip to content
This repository was archived by the owner on May 17, 2024. It is now read-only.

tests: fix presto, add faker classes - #106

Merged
sirupsen merged 3 commits into
masterfrom
presto-and-fakers
Jun 27, 2022
Merged

tests: fix presto, add faker classes#106
sirupsen merged 3 commits into
masterfrom
presto-and-fakers

Conversation

@sirupsen

Copy link
Copy Markdown
Contributor

First set of changes to allow the test suites to be used for benchmarking:

  1. Actually use Presto for inserting/etc., before it was just using the backing Postgres table I think, because it wasn't working properly for me (and fix some Presto related bugs)
  2. Add *Faker classes that are capable of producing an infinite amount of rows for benchmarking
  3. Add pagination for the import into the destination table, required when we do millions of rows
  4. Fix Presto rounding (I think this is supporting of (1) that it wasn't tested the way we thought?)
  5. Add a few more things to make the test and table names nicer

Just trying to split this up so it's easier to review

@sirupsen
sirupsen requested a review from erezsh June 24, 2022 20:51
name = name.replace(r"without time zone", "n_tz")
name = name.replace(r"with time zone", "y_tz")
name = name.replace(r"with local time zone", "y_tz")
name = name.replace(r"timestamp", "ts")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I prefer shortening and never truncating names. That could lead to some nasty, nasty bugs down the line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's a nice idea!

Comment thread tests/common.py Outdated

level = logging.ERROR
if os.environ.get('DEBUG', False):
level = logging.DEBUG

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be more flexible to get the LOG_LEVEL and use getattr with it.

Comment thread tests/test_database_types.py Outdated
import math
import datetime
from decimal import Decimal
from prestodb.exceptions import PrestoUserError

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should avoid this kind of imports.

Instead you can use import_presto() from databases/presto.py where it's needed.

If that's too clumsy, we can store "prestodb" in the Presto insntance, and use it as db.prestodb.exceptions.PrestoUserError.

Comment thread tests/test_database_types.py
name = name.replace(r"without time zone", "n_tz")
name = name.replace(r"with time zone", "y_tz")
name = name.replace(r"with local time zone", "y_tz")
name = name.replace(r"timestamp", "ts")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's a nice idea!

Comment thread tests/test_database_types.py
@erezsh

erezsh commented Jun 25, 2022

Copy link
Copy Markdown
Contributor

Nice one!

@sirupsen
sirupsen force-pushed the presto-and-fakers branch 3 times, most recently from 2d70092 to 6bcf104 Compare June 27, 2022 15:35
Comment thread tests/test_database_types.py
@sirupsen
sirupsen force-pushed the presto-and-fakers branch from 6bcf104 to 5a173fd Compare June 27, 2022 15:37
@sirupsen
sirupsen merged commit 0fe259d into master Jun 27, 2022
@sirupsen
sirupsen deleted the presto-and-fakers branch June 27, 2022 15:42
@sirupsen sirupsen mentioned this pull request Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants