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
merged 3 commits into from
Jun 27, 2022
Merged

tests: fix presto, add faker classes #106

merged 3 commits into from
Jun 27, 2022

Conversation

sirupsen
Copy link
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
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
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!

tests/common.py Outdated

level = logging.ERROR
if os.environ.get('DEBUG', False):
level = logging.DEBUG
Copy link
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.

from decimal import Decimal
from prestodb.exceptions import PrestoUserError
Copy link
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.

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
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!

@erezsh
Copy link
Contributor

erezsh commented Jun 25, 2022

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
@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