fix: error around latest partition in BigQuery #11274
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #11274 +/- ##
==========================================
- Coverage 65.63% 65.62% -0.02%
==========================================
Files 838 839 +1
Lines 39714 39841 +127
Branches 3613 3653 +40
==========================================
+ Hits 26068 26145 +77
- Misses 13538 13595 +57
+ Partials 108 101 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
mistercrunch
force-pushed
the
fix_partition_bug_bigq
branch
2 times, most recently
from
b852a3d to
f099e0e
Compare
mistercrunch
commented
Oct 19, 2020
| @@ -110,7 +110,7 @@ class TableElement extends React.PureComponent { | |||
| /> | |||
| ); | |||
| } | |||
| let latest = Object.entries(table.partitions.latest).map( | |||
| let latest = Object.entries(table.partitions?.latest || []).map( | |||
There was a problem hiding this comment.
This isn't necessary anymore given the issue being addressed in the backend, but is a tiny bit of defensive programming.
mistercrunch
force-pushed
the
fix_partition_bug_bigq
branch
from
4edbc56 to
816d467
Compare
mistercrunch
force-pushed
the
fix_partition_bug_bigq
branch
from
816d467 to
f8dbcae
Compare
dpgaspar
approved these changes
Oct 20, 2020
| # the index's `column_names` == [None] | ||
| # Here we're returning only non-None indexes | ||
| for ix in indexes: | ||
| column_names = ix.get("column_names") or [] |
There was a problem hiding this comment.
nit:
for index in indexes:
column_names = index.get("column_names", [])
There was a problem hiding this comment.
the or is a bit safer in case the key exists with a None value, will stick to it
| normalized_idx = BigQueryEngineSpec.normalize_indexes(indexes) | ||
| self.assertEqual(normalized_idx, []) | ||
|
|
||
| indexes = [{"name": "partition", "column_names": ["dttm"], "unique": False}] |
There was a problem hiding this comment.
we could add another small test with indexes = [{"name": "partition", "column_names": ["dttm", None], "unique": False}]
mistercrunch
force-pushed
the
fix_partition_bug_bigq
branch
from
7fa9092 to
2905eb7
Compare
|
addressed the comments, mergin'! |
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
* fix: error around latest partition in BigQuery * lint * Going with a backend-first approach * fix test * add an extra test
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hit this bug while using BigQ. The SQLAlchemy inspector's
get_indexesreturns a list of columns that is wrong and get the SQL Lab frontend to hard crash.Here's the exact payload I was getting off of
get_indexesfor a particular table:[{'name': 'partition', 'column_names': [None], 'unique': False}]Here I'm adding a BigQuery specific mutation for the indexes that makes sure this doesn't happen. We're assuming that
pybigquerywill fix the root cause in a future version.