Uh oh!
There was an error while loading. Please reload this page.
feat: add support for INTERVAL data type to list_rows - #840
Conversation
tswast
commented
Jul 29, 2021
Some of these commits are from #829 |
Uh oh!
There was an error while loading. Please reload this page.
| minutes = time_sign * int(parsed.group("minutes")) | ||
| seconds = time_sign * int(parsed.group("seconds")) | ||
| fraction = parsed.group("fraction") | ||
| microseconds = time_sign * int(fraction.ljust(6, "0")) if fraction else 0 |
There was a problem hiding this comment.
I can't tell from https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#interval_type if there are limits on any of these fields. 🤷
There was a problem hiding this comment.
I agree. The docs don't really explain much about how this data format actually works. From what is documented and trial and error, I think these are the limits:
Min
- years: -10000
- months: -11 (I think?)
- days: -3660000
- hours: -87840000
- minutes: -59 (I think?)
- seconds: -59 (I think?)
- microseconds: -999999 (I think?)
Max
- years: 10000
- months: 11 (I think?)
- days: 3660000
- hours: 87840000
- minutes: 59 (I think?)
- seconds: 59 (I think?)
- microseconds: 999999 (I think?)
I don't think we'll need any client-side validation for this, but it does remind me that we should have some unit tests that exercise these limits.
| BOOL = ScalarQueryParameterType("BOOL") | ||
| BOOLEAN = ScalarQueryParameterType("BOOL") | ||
| BIGDECIMAL = ScalarQueryParameterType("BIGNUMERIC") | ||
| BIGNUMERIC = ScalarQueryParameterType("BIGNUMERIC") | ||
| BYTES = ScalarQueryParameterType("BYTES") | ||
| INTEGER = ScalarQueryParameterType("INT64") | ||
| INT64 = ScalarQueryParameterType("INT64") | ||
| DATE = ScalarQueryParameterType("DATE") | ||
| DATETIME = ScalarQueryParameterType("DATETIME") | ||
| DECIMAL = ScalarQueryParameterType("NUMERIC") | ||
| FLOAT = ScalarQueryParameterType("FLOAT64") | ||
| FLOAT64 = ScalarQueryParameterType("FLOAT64") | ||
| NUMERIC = ScalarQueryParameterType("NUMERIC") | ||
| BIGNUMERIC = ScalarQueryParameterType("BIGNUMERIC") | ||
| DECIMAL = ScalarQueryParameterType("NUMERIC") | ||
| BIGDECIMAL = ScalarQueryParameterType("BIGNUMERIC") | ||
| BOOLEAN = ScalarQueryParameterType("BOOL") | ||
| BOOL = ScalarQueryParameterType("BOOL") | ||
| GEOGRAPHY = ScalarQueryParameterType("GEOGRAPHY") | ||
| TIMESTAMP = ScalarQueryParameterType("TIMESTAMP") | ||
| DATE = ScalarQueryParameterType("DATE") | ||
| INT64 = ScalarQueryParameterType("INT64") | ||
| INTEGER = ScalarQueryParameterType("INT64") | ||
| NUMERIC = ScalarQueryParameterType("NUMERIC") | ||
| STRING = ScalarQueryParameterType("STRING") | ||
| TIME = ScalarQueryParameterType("TIME") | ||
| DATETIME = ScalarQueryParameterType("DATETIME") | ||
| TIMESTAMP = ScalarQueryParameterType("TIMESTAMP") |
There was a problem hiding this comment.
Did you intend to include INTERVAL?
There was a problem hiding this comment.
I did but then reverted it because the query parameter support is incomplete. I could revert the alphabetization, but I figure we should do that at some point.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
tswast
commented
Aug 5, 2021
I think this is ready, but I'll keep the Do Not Merge label, as I think #829 should go in first. |
Uh oh!
There was an error while loading. Please reload this page.
tswast
commented
Oct 25, 2021
@plamut Did you have any additional thoughts on this? It'd be good to get in before v3. I added some proposed unit tests with trailing |
plamut
left a comment
There was a problem hiding this comment.
Went over this again, it looks fine (just one nit).
Good that we caught the issue in the divmod-based implementation, keeping the regex is the right way to go.
Uh oh!
There was an error while loading. Please reload this page.
* test: refactor `list_rows` tests and add test for scalars * WIP: INTERVAL support * feat: add support for INTERVAL data type to `list_rows` * fix relativedelta construction for non-microseconds * WIP: support INTERVAL query params * remove dead code * INTERVAL not supported in query parameters * revert query parameter changes * add validation error for interval * add unit tests for extreme intervals * add dateutil to intersphinx * use dictionary for intersphinx * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * add test case for trailing . * explicit none * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * truncate nanoseconds * use \d group for digits * use \d for consistency Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Peter Lamut <plamut@users.noreply.github.com>
Implementation of most other features for
INTERVALis blocked by backend changes.Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Towards googleapis/google-cloud-python#15740 🦕