Skip to content

feat: add type_generic and is_dttm to table metadata#14863

Merged
zhaoyongjie merged 6 commits into
apache:masterfrom
zhaoyongjie:get_general_column_type
Jun 3, 2021
Merged

feat: add type_generic and is_dttm to table metadata#14863
zhaoyongjie merged 6 commits into
apache:masterfrom
zhaoyongjie:get_general_column_type

Conversation

@zhaoyongjie
Copy link
Copy Markdown
Member

@zhaoyongjie zhaoyongjie commented May 27, 2021

SUMMARY

For the current the Superset, is_temporal is used to determine if a column is a DateTime column instead of the column type (Only the columns marked with is_temporal can be used for time picker). Superset doesn't mark a time column as is_temporal when creating a new dataset or syncing an existing dataset.

This PR introduce is_dttm and general_column fields to the dataset metadata. so that it can be mark a DateTime column with is_temporal.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

update existing columns

update.exist.columns.mp4

add new columns

add.new.columns.mp4

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue: fixes [Explore]'Is temporal' checkbox should take effect #14754
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2021

Codecov Report

Merging #14863 (133238b) into master (9f54231) will decrease coverage by 0.02%.
The diff coverage is 68.25%.

❗ Current head 133238b differs from pull request most recent head e2f17d1. Consider uploading reports for the commit e2f17d1 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14863      +/-   ##
==========================================
- Coverage   77.38%   77.35%   -0.03%     
==========================================
  Files         963      965       +2     
  Lines       49245    49438     +193     
  Branches     6197     6247      +50     
==========================================
+ Hits        38110    38245     +135     
- Misses      10934    10992      +58     
  Partials      201      201              
Flag Coverage Δ
mysql 81.63% <85.71%> (+0.04%) ⬆️
postgres 81.66% <85.71%> (+0.04%) ⬆️
python 81.74% <87.14%> (+0.04%) ⬆️
sqlite 81.27% <87.14%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...erset-frontend/src/SqlLab/components/ResultSet.tsx 67.42% <0.00%> (-1.57%) ⬇️
superset-frontend/src/components/Form/FormItem.tsx 100.00% <ø> (ø)
...uperset-frontend/src/components/Menu/MenuRight.tsx 93.75% <ø> (ø)
superset-frontend/src/components/Tabs/Tabs.tsx 96.55% <ø> (ø)
superset-frontend/src/constants.ts 100.00% <ø> (ø)
superset-frontend/src/dashboard/actions/hydrate.js 1.72% <0.00%> (-0.02%) ⬇️
.../components/Header/HeaderActionsDropdown/index.jsx 68.42% <ø> (ø)
...veFilters/FilterBar/FilterControls/FilterValue.tsx 67.44% <ø> (ø)
...mponents/nativeFilters/FiltersConfigModal/utils.ts 67.94% <ø> (-0.81%) ⬇️
...nd/src/dashboard/containers/DashboardComponent.jsx 84.84% <ø> (ø)
... and 57 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f54231...e2f17d1. Read the comment docs.

@zhaoyongjie zhaoyongjie requested a review from villebro May 27, 2021 12:30
Comment thread .pylintrc
# no Warning level messages displayed, use"--disable=all --enable=classes
# --disable=W"
disable=long-builtin,dict-view-method,intern-builtin,suppressed-message,no-absolute-import,unpacking-in-except,apply-builtin,delslice-method,indexing-exception,old-raise-syntax,print-statement,cmp-builtin,reduce-builtin,useless-suppression,coerce-method,input-builtin,cmp-method,raw_input-builtin,nonzero-method,backtick,basestring-builtin,setslice-method,reload-builtin,oct-method,map-builtin-not-iterating,execfile-builtin,old-octal-literal,zip-builtin-not-iterating,buffer-builtin,getslice-method,metaclass-assignment,xrange-builtin,long-suffix,round-builtin,range-builtin-not-iterating,next-method-called,parameter-unpacking,unicode-builtin,unichr-builtin,import-star-module-level,raising-string,filter-builtin-not-iterating,using-cmp-argument,coerce-builtin,file-builtin,old-division,hex-method,missing-docstring,too-many-lines,ungrouped-imports,import-outside-toplevel,raise-missing-from,super-with-arguments,bad-option-value,too-few-public-methods
disable=long-builtin,dict-view-method,intern-builtin,suppressed-message,no-absolute-import,unpacking-in-except,apply-builtin,delslice-method,indexing-exception,old-raise-syntax,print-statement,cmp-builtin,reduce-builtin,useless-suppression,coerce-method,input-builtin,cmp-method,raw_input-builtin,nonzero-method,backtick,basestring-builtin,setslice-method,reload-builtin,oct-method,map-builtin-not-iterating,execfile-builtin,old-octal-literal,zip-builtin-not-iterating,buffer-builtin,getslice-method,metaclass-assignment,xrange-builtin,long-suffix,round-builtin,range-builtin-not-iterating,next-method-called,parameter-unpacking,unicode-builtin,unichr-builtin,import-star-module-level,raising-string,filter-builtin-not-iterating,using-cmp-argument,coerce-builtin,file-builtin,old-division,hex-method,missing-docstring,too-many-lines,ungrouped-imports,import-outside-toplevel,raise-missing-from,super-with-arguments,bad-option-value,too-few-public-methods,too-many-locals
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

disable too-many-locals lint rule. There are too many ignore in the codebase for this rule.

@zhaoyongjie zhaoyongjie changed the title feat: add general column type to table metadata feat: add generic_type and is_dttm to table metadata May 28, 2021
Copy link
Copy Markdown
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Small comment about naming. I also wonder if we should start deprecating the use of is_dttm and start promoting the use of type_generic?

Comment thread superset/connectors/sqla/models.py Outdated
@zhaoyongjie
Copy link
Copy Markdown
Member Author

Small comment about naming. I also wonder if we should start deprecating the use of is_dttm and start promoting the use of type_generic?

I think in the explore page should return type_generic(derived from type) instead of is_dttm or type. but in the sqllab should return database original column type.

@zhaoyongjie zhaoyongjie requested a review from villebro June 2, 2021 10:17
Comment thread superset-frontend/src/datasource/DatasourceEditor.jsx Outdated
@zhaoyongjie zhaoyongjie changed the title feat: add generic_type and is_dttm to table metadata feat: add type_generic and is_dttm to table metadata Jun 2, 2021
Copy link
Copy Markdown
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM

@zhaoyongjie zhaoyongjie merged commit e6bc7c9 into apache:master Jun 3, 2021
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jun 6, 2021
* upstream/master: (68 commits)
  fix typos (apache#14950)
  docs: fix custom oauth config (apache#14997)
  fix: apply template_params on external_metadata (apache#14996)
  fix: toggle fullscreen on the dashboard (apache#14979)
  feat(native-filters): add markers and number formatter + simple tests (apache#14981)
  fix(native-filters): Fix "undefined" error after editing a filter (apache#14984)
  fix(native-filters): remove implied fetch predicate (apache#14982)
  fix(native-filters): update cascaded filter state on change (apache#14980)
  fix(filter box): replace freeform where clause with ilike (apache#14900)
  refactor: Convert TableElement.jsx component from class to functional with hooks (apache#14830)
  fix: renamed sqllab filters to _filters (apache#14971)
  feat(native-filters): apply cascading without instant filtering (apache#14966)
  chore: bump superset-ui to 0.17.53 (apache#14968)
  fix(native-filters): cascading filters not rendering in tab (apache#14964)
  feat: add type_generic and is_dttm to table metadata (apache#14863)
  additional safeguard (apache#14953)
  feat: Adding FORCE_SSL as feature flag in config.py (apache#14934)
  feat(dashboard/native-filters): Hide filters out of scope of current tab (apache#14933)
  fix: time parser truncate to first day of year/month (apache#14945)
  fix: is_temporal should overwrite is_dttm (apache#14894)
  ...
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 First shipped in 1.3.0 labels Mar 12, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 1.3.0 First shipped in 1.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants