You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/11/24 18:55:06 UTC

[GitHub] [superset] betodealmeida opened a new pull request #17543: feat: new dataset/table/column models

betodealmeida opened a new pull request #17543:
URL: https://github.com/apache/superset/pull/17543


   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   WIP
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-1032331672


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] pkdotson commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
pkdotson commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r813444812



##########
File path: tests/unit_tests/columns/__init__.py
##########
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one

Review comment:
       is this file necessary?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] craig-rueda commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r765175838



##########
File path: superset/connectors/sqla/models.py
##########
@@ -71,12 +72,14 @@
 from sqlalchemy.sql.selectable import Alias, TableClause
 
 from superset import app, db, is_feature_enabled, security_manager
+from superset.columns.models import Column as NewColumn
 from superset.common.db_query_status import QueryStatus
 from superset.connectors.base.models import BaseColumn, BaseDatasource, BaseMetric
 from superset.connectors.sqla.utils import (
     get_physical_table_metadata,
     get_virtual_table_metadata,
 )
+from superset.datasets.models import Dataset as NewDataset

Review comment:
       Maybe alias this as something like `DatasetV2`? I get that it's "new and shiny" but it won't always be the case :).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2fcdeef) into [master](https://codecov.io/gh/apache/superset/commit/dafc841e223c0f01092a2e116888a3304142e1b8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dafc841) will **increase** coverage by `0.10%`.
   > The diff coverage is `98.79%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   + Coverage   66.21%   66.31%   +0.10%     
   ==========================================
     Files        1633     1638       +5     
     Lines       63210    63454     +244     
     Branches     6409     6409              
   ==========================================
   + Hits        41852    42079     +227     
   - Misses      19698    19715      +17     
     Partials     1660     1660              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.45% <73.09%> (+0.16%)` | :arrow_up: |
   | mysql | `?` | |
   | postgres | `81.73% <98.79%> (+0.13%)` | :arrow_up: |
   | presto | `52.29% <73.09%> (+0.16%)` | :arrow_up: |
   | python | `82.12% <98.79%> (+0.08%)` | :arrow_up: |
   | sqlite | `81.42% <98.79%> (+0.13%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.04% <98.02%> (+1.28%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `97.18% <100.00%> (+0.14%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.90% <100.00%> (+0.24%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `94.40% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dafc841...2fcdeef](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2fcdeef) into [master](https://codecov.io/gh/apache/superset/commit/dafc841e223c0f01092a2e116888a3304142e1b8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dafc841) will **increase** coverage by `0.12%`.
   > The diff coverage is `98.79%`.
   
   > :exclamation: Current head 2fcdeef differs from pull request most recent head 142d625. Consider uploading reports for the commit 142d625 to get more accurate results
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   + Coverage   66.21%   66.33%   +0.12%     
   ==========================================
     Files        1633     1638       +5     
     Lines       63210    63454     +244     
     Branches     6409     6409              
   ==========================================
   + Hits        41852    42092     +240     
   - Misses      19698    19702       +4     
     Partials     1660     1660              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.45% <73.09%> (+0.16%)` | :arrow_up: |
   | mysql | `81.68% <98.79%> (+0.13%)` | :arrow_up: |
   | postgres | `81.73% <98.79%> (+0.13%)` | :arrow_up: |
   | presto | `52.29% <73.09%> (+0.16%)` | :arrow_up: |
   | python | `82.16% <98.79%> (+0.12%)` | :arrow_up: |
   | sqlite | `81.42% <98.79%> (+0.13%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.04% <98.02%> (+1.28%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `97.18% <100.00%> (+0.14%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.90% <100.00%> (+0.24%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `94.40% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `98.11% <0.00%> (-1.89%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dafc841...142d625](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2fcdeef) into [master](https://codecov.io/gh/apache/superset/commit/dafc841e223c0f01092a2e116888a3304142e1b8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dafc841) will **increase** coverage by `0.12%`.
   > The diff coverage is `98.79%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   + Coverage   66.21%   66.33%   +0.12%     
   ==========================================
     Files        1633     1638       +5     
     Lines       63210    63454     +244     
     Branches     6409     6409              
   ==========================================
   + Hits        41852    42092     +240     
   - Misses      19698    19702       +4     
     Partials     1660     1660              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.45% <73.09%> (+0.16%)` | :arrow_up: |
   | mysql | `81.68% <98.79%> (+0.13%)` | :arrow_up: |
   | postgres | `81.73% <98.79%> (+0.13%)` | :arrow_up: |
   | presto | `52.29% <73.09%> (+0.16%)` | :arrow_up: |
   | python | `82.16% <98.79%> (+0.12%)` | :arrow_up: |
   | sqlite | `81.42% <98.79%> (+0.13%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.04% <98.02%> (+1.28%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `97.18% <100.00%> (+0.14%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.90% <100.00%> (+0.24%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `94.40% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `98.11% <0.00%> (-1.89%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dafc841...2fcdeef](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-1050734209


   This is impressive work @betodealmeida ! 🚀 Talk about swapping out the airplane's engines mid flight! ✈️ 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r812463533



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.

Review comment:
       @dpgaspar @john-bodley  any thoughts on this?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-998899043


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida merged pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida merged pull request #17543:
URL: https://github.com/apache/superset/pull/17543


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2fcdeef) into [master](https://codecov.io/gh/apache/superset/commit/dafc841e223c0f01092a2e116888a3304142e1b8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dafc841) will **decrease** coverage by `0.06%`.
   > The diff coverage is `98.79%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   - Coverage   66.21%   66.14%   -0.07%     
   ==========================================
     Files        1633     1638       +5     
     Lines       63210    63454     +244     
     Branches     6409     6409              
   ==========================================
   + Hits        41852    41974     +122     
   - Misses      19698    19820     +122     
     Partials     1660     1660              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.45% <73.09%> (+0.16%)` | :arrow_up: |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `52.29% <73.09%> (+0.16%)` | :arrow_up: |
   | python | `81.78% <98.79%> (-0.26%)` | :arrow_down: |
   | sqlite | `81.42% <98.79%> (+0.13%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.57% <98.02%> (+0.80%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `97.18% <100.00%> (+0.14%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.90% <100.00%> (+0.24%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `94.40% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.37% <0.00%> (-56.87%)` | :arrow_down: |
   | ... and [21 more](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dafc841...2fcdeef](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r813445993



##########
File path: tests/unit_tests/columns/__init__.py
##########
@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one

Review comment:
       It is... in theory in Python 3 empty `__init__py` files are not needed, but in practice we had a lot of problems when we omit them.
   
   For `pytest` in particular, if you have files with the same name in different directories inside your tests, like:
   
   - `tests/unit_tests/datasets/test_model.py`
   - `tests/unit_tests/tables/test_model.py`
   
   Then you *need* to add the `__init__.py` file to the subdirectories, otherwise pytest will only read one of them.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-1011282922


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r813452590



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)

Review comment:
       Since this is a new model, I don't see any reason why we couldn't add it in here since it wouldn't break anything, but yes I agree on a SIP regarding an overall change toward that methodology. If that's the blocker, then that makes sense. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-994560679


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r766145860



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"

Review comment:
       Out of interest what does the `sl` prefix represent? Additionally is it necessary?

##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+
+    # We use ``sa.Text`` for these attributes because (1) in modern databases the
+    # performance is the same as ``VARCHAR``[1] and (2) because some table names can be
+    # **really** long (eg, Google Sheets URLs).
+    #
+    # [1] https://www.postgresql.org/docs/9.1/datatype-character.html
+    name = sa.Column(sa.Text)
+    type = sa.Column(sa.Text)
+
+    # Columns are defined by expressions. For tables, these are the actual columns names,
+    # and should match the ``name`` attribute. For datasets, these can be any valid SQL
+    # expression. If the SQL expression is an aggregation the column is a metric,
+    # otherwise it's a computed column.
+    expression = sa.Column(sa.Text)
+
+    # Does the expression point directly to a physical column?
+    is_physical = sa.Column(sa.Boolean, default=True)
+
+    # Additional metadata describing the column.
+    description = sa.Column(sa.Text)
+    warning_text = sa.Column(sa.Text)
+    units = sa.Column(sa.Text)
+
+    # Is this a time column? Useful for plotting time series.
+    is_temporal = sa.Column(sa.Boolean, default=False)
+
+    # Is this a spatial column? This could be leveraged in the future for spatial
+    # visualizations.
+    is_spatial = sa.Column(sa.Boolean, default=False)
+
+    # Is this column a partition? Useful for scheduling queries and previewing the latest
+    # data.
+    is_partition = sa.Column(sa.Boolean, default=False)
+
+    # Is this column an aggregation (metric)?
+    is_aggregation = sa.Column(sa.Boolean, default=False)
+
+    # Assuming the column is an aggregation, is it additive? Useful for determining which
+    # aggregations can be done on the metric. Eg, ``COUNT(DISTINCT user_id)`` is not
+    # additive, so it shouldn't be used in a ``SUM``.
+    is_additive = sa.Column(sa.Boolean, default=False)
+
+    # Is an increase good? Useful for displaying the results of A/B tests, or setting up
+    # alerts. Eg, this is true for "revenue", but false for "latency".
+    increase_good = sa.Column(sa.Boolean, default=True)

Review comment:
       Maybe "desired" as opposed to "good"? Also for boolean columns I find the `is_` or `has_` prefix is helpful so `is_increased_desired`.

##########
File path: superset/tables/models.py
##########
@@ -0,0 +1,88 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Table model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "table" in a given database -- either a physical table or a view. In
+addition to a table, new models for columns, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+from typing import List
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+from sqlalchemy.orm import backref, relationship
+from sqlalchemy.schema import UniqueConstraint
+
+from superset.columns.models import Column
+from superset.models.core import Database
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+association_table = sa.Table(
+    "sl_table_columns",
+    Model.metadata,  # pylint: disable=no-member
+    sa.Column("table_id", sa.ForeignKey("sl_tables.id")),
+    sa.Column("column_id", sa.ForeignKey("sl_columns.id")),
+)
+
+
+class Table(Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin):
+    """
+    A table/view in a database.
+    """
+
+    __tablename__ = "sl_tables"
+
+    # Note this uniqueness constraint is not part of the physical schema, i.e., it does

Review comment:
       Thanks for including this comment. I know it's something which has tripped us up in the past.

##########
File path: tests/unit_tests/tables/test_models.py
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel, unused-argument
+
+from sqlalchemy.orm.session import Session
+
+
+def test_table_model(app_context: None, session: Session) -> None:
+    """
+    Test basic attributes of a ``Table``.
+    """
+    from superset.columns.models import Column
+    from superset.models.core import Database
+    from superset.tables.models import Table
+
+    engine = session.get_bind()
+    Table.metadata.create_all(engine)  # pylint: disable=no-member
+
+    table = Table(
+        name="my_table",
+        schema="my_schema",
+        catalog="my_catalog",
+        database=Database(database_name="my_database", sqlalchemy_uri="test://"),
+        columns=[Column(name="ds", type="TIMESTAMP", expression="ds",)],
+    )
+    session.add(table)
+    session.flush()
+
+    assert table.id == 1
+    assert table.uuid is not None
+

Review comment:
       ```suggestion
   ```

##########
File path: tests/unit_tests/tables/test_models.py
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel, unused-argument
+
+from sqlalchemy.orm.session import Session
+
+
+def test_table_model(app_context: None, session: Session) -> None:
+    """
+    Test basic attributes of a ``Table``.
+    """
+    from superset.columns.models import Column
+    from superset.models.core import Database
+    from superset.tables.models import Table
+
+    engine = session.get_bind()
+    Table.metadata.create_all(engine)  # pylint: disable=no-member
+
+    table = Table(
+        name="my_table",
+        schema="my_schema",
+        catalog="my_catalog",
+        database=Database(database_name="my_database", sqlalchemy_uri="test://"),
+        columns=[Column(name="ds", type="TIMESTAMP", expression="ds",)],
+    )
+    session.add(table)
+    session.flush()
+
+    assert table.id == 1
+    assert table.uuid is not None
+
+    assert table.database_id == 1
+

Review comment:
       ```suggestion
   ```

##########
File path: superset/connectors/sqla/models.py
##########
@@ -1790,23 +1806,448 @@ def before_update(
             raise Exception(get_dataset_exist_error_msg(target.full_name))
 
     @staticmethod
-    def update_table(
-        _mapper: Mapper, _connection: Connection, obj: Union[SqlMetric, TableColumn]
+    def update_table(  # pylint: disable=unused-argument
+        mapper: Mapper, connection: Connection, target: Union[SqlMetric, TableColumn]
     ) -> None:
         """
         Forces an update to the table's changed_on value when a metric or column on the
         table is updated. This busts the cache key for all charts that use the table.
 
-        :param _mapper: Unused.
-        :param _connection: Unused.
-        :param obj: The metric or column that was updated.
+        :param mapper: Unused.
+        :param connection: Unused.
+        :param target: The metric or column that was updated.
+        """
+        inspector = inspect(target)
+        session = inspector.session
+
+        # get DB-specific conditional quoter for expressions that point to columns or
+        # table names
+        database = (
+            target.table.database
+            or session.query(Database).filter_by(id=target.database_id).one()
+        )
+        engine = database.get_sqla_engine(schema=target.table.schema)
+        conditional_quote = engine.dialect.identifier_preparer.quote
+
+        session.execute(update(SqlaTable).where(SqlaTable.id == target.table.id))
+
+        # update ``Column`` model as well
+        dataset = (
+            session.query(NewDataset).filter_by(sqlatable_id=target.table.id).one()
+        )
+
+        if isinstance(target, TableColumn):
+            columns = [
+                column
+                for column in dataset.columns
+                if column.name == target.column_name
+            ]
+            if not columns:
+                return
+
+            column = columns[0]
+            extra_json = json.loads(target.extra or "{}")
+            for attr in {"groupby", "filterable", "verbose_name", "python_date_format"}:
+                value = getattr(target, attr)
+                if value:
+                    extra_json[attr] = value
+
+            column.name = target.column_name
+            column.type = target.type or "Unknown"
+            column.expression = target.expression or conditional_quote(
+                target.column_name
+            )
+            column.description = target.description
+            column.is_temporal = target.is_dttm
+            column.is_physical = target.expression is None
+            column.extra_json = json.dumps(extra_json) if extra_json else None
+
+        else:  # SqlMetric
+            columns = [
+                column
+                for column in dataset.columns
+                if column.name == target.metric_name
+            ]
+            if not columns:
+                return
+
+            column = columns[0]
+            extra_json = json.loads(target.extra or "{}")
+            for attr in {"verbose_name", "metric_type", "d3format"}:
+                value = getattr(target, attr)
+                if value:
+                    extra_json[attr] = value
+
+            is_additive = (
+                target.metric_type
+                and target.metric_type.lower() in ADDITIVE_METRIC_TYPES
+            )
+
+            column.name = target.metric_name
+            column.expression = target.expression
+            column.warning_text = target.warning_text
+            column.description = target.description
+            column.is_additive = is_additive
+            column.extra_json = json.dumps(extra_json) if extra_json else None
+
+    @staticmethod
+    def after_insert(  # pylint: disable=too-many-locals
+        mapper: Mapper, connection: Connection, target: "SqlaTable",
+    ) -> None:
+        """
+        Shadow write the dataset to new models.

Review comment:
       I think the dual write approach is a good call.

##########
File path: tests/unit_tests/tables/test_models.py
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel, unused-argument
+
+from sqlalchemy.orm.session import Session
+
+
+def test_table_model(app_context: None, session: Session) -> None:
+    """
+    Test basic attributes of a ``Table``.
+    """
+    from superset.columns.models import Column
+    from superset.models.core import Database
+    from superset.tables.models import Table
+
+    engine = session.get_bind()
+    Table.metadata.create_all(engine)  # pylint: disable=no-member
+
+    table = Table(
+        name="my_table",
+        schema="my_schema",
+        catalog="my_catalog",
+        database=Database(database_name="my_database", sqlalchemy_uri="test://"),
+        columns=[Column(name="ds", type="TIMESTAMP", expression="ds",)],
+    )
+    session.add(table)
+    session.flush()
+
+    assert table.id == 1
+    assert table.uuid is not None
+
+    assert table.database_id == 1
+
+    assert table.catalog == "my_catalog"
+    assert table.schema == "my_schema"
+    assert table.name == "my_table"
+

Review comment:
       ```suggestion
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r765311996



##########
File path: superset/connectors/sqla/models.py
##########
@@ -1790,23 +1806,448 @@ def before_update(
             raise Exception(get_dataset_exist_error_msg(target.full_name))
 
     @staticmethod
-    def update_table(
-        _mapper: Mapper, _connection: Connection, obj: Union[SqlMetric, TableColumn]
+    def update_table(  # pylint: disable=unused-argument
+        mapper: Mapper, connection: Connection, target: Union[SqlMetric, TableColumn]
     ) -> None:
         """
         Forces an update to the table's changed_on value when a metric or column on the
         table is updated. This busts the cache key for all charts that use the table.
 
-        :param _mapper: Unused.
-        :param _connection: Unused.
-        :param obj: The metric or column that was updated.
+        :param mapper: Unused.

Review comment:
       I left it in the old model because it will go away with the old model — this is only needed while both exist. But I agree it's hard to read, I'll break it down. There's also some duplicate logic, but since this is temporary code that's going away I thought it would be ok.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r767010769



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+
+    # We use ``sa.Text`` for these attributes because (1) in modern databases the
+    # performance is the same as ``VARCHAR``[1] and (2) because some table names can be
+    # **really** long (eg, Google Sheets URLs).
+    #
+    # [1] https://www.postgresql.org/docs/9.1/datatype-character.html
+    name = sa.Column(sa.Text)
+    type = sa.Column(sa.Text)
+
+    # Columns are defined by expressions. For tables, these are the actual columns names,

Review comment:
       I'm hoping that having `expression` even for physical columns and tables will make things easier because of the consistency. One thing the new model does is automatically quote table/column names that are also identifiers; as an example, we'd have:
   
   ```python
   column = Column(
       name="select",
       expression="`select`",  # or "select", depending on the DB
       ...
   )
   ```
   
   So to select that column we just need to use its `expression`. Otherwise, we'd have to check if `expression` is null, and then potentially encode the name in order to select that column.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-979862926


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (20def3e) into [master](https://codecov.io/gh/apache/superset/commit/7353a2bd75ae5c76458615443f530a688e78db1c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7353a2b) will **increase** coverage by `0.14%`.
   > The diff coverage is `98.34%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   + Coverage   76.81%   76.96%   +0.14%     
   ==========================================
     Files        1049     1067      +18     
     Lines       56653    57233     +580     
     Branches     7847     7847              
   ==========================================
   + Hits        43517    44048     +531     
   - Misses      12883    12932      +49     
     Partials      253      253              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `81.52% <98.34%> (?)` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `81.81% <98.34%> (?)` | |
   | python | `82.28% <98.34%> (+0.18%)` | :arrow_up: |
   | sqlite | `81.63% <98.34%> (-0.07%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.68% <97.33%> (+2.88%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `96.72% <100.00%> (+0.19%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.83% <100.00%> (+0.25%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.90% <100.00%> (+0.04%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | ... and [66 more](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7353a2b...20def3e](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] craig-rueda commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r765176831



##########
File path: superset/migrations/versions/b8d3a24d9131_new_dataset_models.py
##########
@@ -0,0 +1,173 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""new dataset models
+
+Revision ID: b8d3a24d9131
+Revises: aea15018d53b
+Create Date: 2021-11-11 16:41:53.266965
+
+"""
+
+from uuid import uuid4
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy_utils import UUIDType
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable

Review comment:
       Don't import models directly in migrations. If (when) we ever update this model, the migrations might (will) break.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] craig-rueda commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r765174201



##########
File path: superset/connectors/sqla/models.py
##########
@@ -1790,23 +1806,448 @@ def before_update(
             raise Exception(get_dataset_exist_error_msg(target.full_name))
 
     @staticmethod
-    def update_table(
-        _mapper: Mapper, _connection: Connection, obj: Union[SqlMetric, TableColumn]
+    def update_table(  # pylint: disable=unused-argument
+        mapper: Mapper, connection: Connection, target: Union[SqlMetric, TableColumn]
     ) -> None:
         """
         Forces an update to the table's changed_on value when a metric or column on the
         table is updated. This busts the cache key for all charts that use the table.
 
-        :param _mapper: Unused.
-        :param _connection: Unused.
-        :param obj: The metric or column that was updated.
+        :param mapper: Unused.

Review comment:
       Would it make sense to pull this hook logic out into each "New" model's respective classes? This is a rather large block and could use a little breakup :).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-992908734


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r813451461



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.

Review comment:
       It looks like @willbarrett has models at the top level in that Sip, but I don't have much more context than that. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-985737695


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-1005324448


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r789271892



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.

Review comment:
       Other than the connectors, don't we normally put the model files in the superset/models/* folders?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] dpgaspar commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r772978726



##########
File path: superset/columns/schemas.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Schema for the column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+from marshmallow_sqlalchemy import SQLAlchemyAutoSchema
+
+from superset.columns.models import Column
+
+
+class ColumnSchema(SQLAlchemyAutoSchema):

Review comment:
       Where is this used? are you planning to include it on a followup PR?
   It's a fast and dynamic way of generating Marshmallow schemas but can lack some custom validations and will not have descriptions for example, those are a nice to have on OpenAPI spec




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-999838274


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-992970841


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r789088705



##########
File path: tests/unit_tests/columns/test_models.py
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# pylint: disable=import-outside-toplevel, unused-argument
+
+from sqlalchemy.orm.session import Session
+
+
+def test_column_model(app_context: None, session: Session) -> None:
+    """
+    Test basic attributes of a ``Column``.
+    """
+    from superset.columns.models import Column
+
+    engine = session.get_bind()
+    Column.metadata.create_all(engine)  # pylint: disable=no-member
+
+    column = Column(name="ds", type="TIMESTAMP", expression="ds",)
+
+    session.add(column)
+    session.flush()
+
+    assert column.id == 1
+    assert column.uuid is not None
+
+    assert column.name == "ds"
+    assert column.type == "TIMESTAMP"
+    assert column.expression == "ds"
+
+    # test that default values are set correctly
+    assert column.description is None
+    assert column.warning_text is None
+    assert column.units is None

Review comment:
       should this be singular `unit`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r812463689



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)

Review comment:
       I think we would need a SIP and a major release for that, no?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r812463408



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.

Review comment:
       I tried to have this aligned with https://github.com/apache/superset/issues/9077. `superset/models/` came before that (I think). But I have no strong preference here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r813478095



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)

Review comment:
       Ah, I see your point. But the idea is that at least initially the APIs would be unchanged, we'd just reroute the backend to read from the new models instead of the old ones.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-995319582


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r765312448



##########
File path: superset/connectors/sqla/models.py
##########
@@ -71,12 +72,14 @@
 from sqlalchemy.sql.selectable import Alias, TableClause
 
 from superset import app, db, is_feature_enabled, security_manager
+from superset.columns.models import Column as NewColumn
 from superset.common.db_query_status import QueryStatus
 from superset.connectors.base.models import BaseColumn, BaseDatasource, BaseMetric
 from superset.connectors.sqla.utils import (
     get_physical_table_metadata,
     get_virtual_table_metadata,
 )
+from superset.datasets.models import Dataset as NewDataset

Review comment:
       After this PR the plan is to get rid of the old models, so this will go away before we have v3.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r789272986



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)

Review comment:
       not a blocker, but more of a question.. when do we want to start using uuids instead of auto-incrementing integers for  public-facing ids?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-1020441531


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] ktmud commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r839283341



##########
File path: superset/tables/models.py
##########
@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Table model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "table" in a given database -- either a physical table or a view. In
+addition to a table, new models for columns, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+from typing import List
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+from sqlalchemy.orm import backref, relationship
+from sqlalchemy.schema import UniqueConstraint
+
+from superset.columns.models import Column
+from superset.models.core import Database
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+association_table = sa.Table(
+    "sl_table_columns",
+    Model.metadata,  # pylint: disable=no-member
+    sa.Column("table_id", sa.ForeignKey("sl_tables.id")),
+    sa.Column("column_id", sa.ForeignKey("sl_columns.id")),
+)
+
+
+class Table(Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin):
+    """
+    A table/view in a database.
+    """
+
+    __tablename__ = "sl_tables"
+
+    # Note this uniqueness constraint is not part of the physical schema, i.e., it does
+    # not exist in the migrations. The reason it does not physically exist is MySQL,
+    # PostgreSQL, etc. have a different interpretation of uniqueness when it comes to NULL
+    # which is problematic given the catalog and schema are optional.
+    __table_args__ = (UniqueConstraint("database_id", "catalog", "schema", "name"),)

Review comment:
       @betodealmeida It's also not possible to apply this constraint in MySQL because TEXT cannot be used in index keys without specifying a length: https://stackoverflow.com/questions/1827063/mysql-error-key-specification-without-a-key-length




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r767010769



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+
+    # We use ``sa.Text`` for these attributes because (1) in modern databases the
+    # performance is the same as ``VARCHAR``[1] and (2) because some table names can be
+    # **really** long (eg, Google Sheets URLs).
+    #
+    # [1] https://www.postgresql.org/docs/9.1/datatype-character.html
+    name = sa.Column(sa.Text)
+    type = sa.Column(sa.Text)
+
+    # Columns are defined by expressions. For tables, these are the actual columns names,

Review comment:
       I'm hoping that having `expression` even for physical columns and tables will make things easier because of the consistency. One thing the new model does is automatically quote table/column names that are also identifiers; as an example, we'd have:
   
   ```python
   column = Column(
       name="select",
       expression="`select`",  # or "select", depending on the DB
       ...
   )
   ```
   
   So to select that column we just need to use its `expression`. Otherwise, we'd have to check if `expression` is null, and then potentially encode the name in order to select that column.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2fcdeef) into [master](https://codecov.io/gh/apache/superset/commit/dafc841e223c0f01092a2e116888a3304142e1b8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dafc841) will **increase** coverage by `0.09%`.
   > The diff coverage is `98.79%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   + Coverage   66.21%   66.30%   +0.09%     
   ==========================================
     Files        1633     1638       +5     
     Lines       63210    63454     +244     
     Branches     6409     6409              
   ==========================================
   + Hits        41852    42075     +223     
   - Misses      19698    19719      +21     
     Partials     1660     1660              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `52.45% <73.09%> (+0.16%)` | :arrow_up: |
   | mysql | `?` | |
   | postgres | `81.72% <98.79%> (+0.11%)` | :arrow_up: |
   | presto | `52.29% <73.09%> (+0.16%)` | :arrow_up: |
   | python | `82.11% <98.79%> (+0.07%)` | :arrow_up: |
   | sqlite | `81.42% <98.79%> (+0.13%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.04% <98.02%> (+1.28%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `97.18% <100.00%> (+0.14%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.90% <100.00%> (+0.24%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `94.40% <100.00%> (+0.02%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/common/utils/dataframe\_utils.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3V0aWxzL2RhdGFmcmFtZV91dGlscy5weQ==) | `85.71% <0.00%> (-7.15%)` | :arrow_down: |
   | ... and [8 more](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [dafc841...2fcdeef](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] john-bodley commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r766155798



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+
+    # We use ``sa.Text`` for these attributes because (1) in modern databases the
+    # performance is the same as ``VARCHAR``[1] and (2) because some table names can be
+    # **really** long (eg, Google Sheets URLs).
+    #
+    # [1] https://www.postgresql.org/docs/9.1/datatype-character.html
+    name = sa.Column(sa.Text)
+    type = sa.Column(sa.Text)
+
+    # Columns are defined by expressions. For tables, these are the actual columns names,

Review comment:
       This is the one thing I find somewhat atypical, i.e., that a physical table/view column would require require and expression which needs to match the name. Why not just make `expression` nullable in that case and thus the column would only represent actual SQL expressions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r766234041



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"
+
+    id = sa.Column(sa.Integer, primary_key=True)
+
+    # We use ``sa.Text`` for these attributes because (1) in modern databases the
+    # performance is the same as ``VARCHAR``[1] and (2) because some table names can be
+    # **really** long (eg, Google Sheets URLs).
+    #
+    # [1] https://www.postgresql.org/docs/9.1/datatype-character.html
+    name = sa.Column(sa.Text)
+    type = sa.Column(sa.Text)
+
+    # Columns are defined by expressions. For tables, these are the actual columns names,
+    # and should match the ``name`` attribute. For datasets, these can be any valid SQL
+    # expression. If the SQL expression is an aggregation the column is a metric,
+    # otherwise it's a computed column.
+    expression = sa.Column(sa.Text)
+
+    # Does the expression point directly to a physical column?
+    is_physical = sa.Column(sa.Boolean, default=True)
+
+    # Additional metadata describing the column.
+    description = sa.Column(sa.Text)
+    warning_text = sa.Column(sa.Text)
+    units = sa.Column(sa.Text)
+
+    # Is this a time column? Useful for plotting time series.
+    is_temporal = sa.Column(sa.Boolean, default=False)
+
+    # Is this a spatial column? This could be leveraged in the future for spatial
+    # visualizations.
+    is_spatial = sa.Column(sa.Boolean, default=False)
+
+    # Is this column a partition? Useful for scheduling queries and previewing the latest
+    # data.
+    is_partition = sa.Column(sa.Boolean, default=False)
+
+    # Is this column an aggregation (metric)?
+    is_aggregation = sa.Column(sa.Boolean, default=False)
+
+    # Assuming the column is an aggregation, is it additive? Useful for determining which
+    # aggregations can be done on the metric. Eg, ``COUNT(DISTINCT user_id)`` is not
+    # additive, so it shouldn't be used in a ``SUM``.
+    is_additive = sa.Column(sa.Boolean, default=False)
+
+    # Is an increase good? Useful for displaying the results of A/B tests, or setting up
+    # alerts. Eg, this is true for "revenue", but false for "latency".
+    increase_good = sa.Column(sa.Boolean, default=True)

Review comment:
       Ah, good point, I'll change it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r766233902



##########
File path: superset/columns/models.py
##########
@@ -0,0 +1,95 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+import sqlalchemy as sa
+from flask_appbuilder import Model
+
+from superset.models.helpers import (
+    AuditMixinNullable,
+    ExtraJSONMixin,
+    ImportExportMixin,
+)
+
+
+class Column(
+    Model, AuditMixinNullable, ExtraJSONMixin, ImportExportMixin,
+):
+    """
+    A "column".
+
+    The definition of column here is overloaded: it can represent a physical column in a
+    database relation (table or view); a computed/derived column on a dataset; or an
+    aggregation expression representing a metric.
+    """
+
+    __tablename__ = "sl_columns"

Review comment:
       Semantic layer. I wanted to use `columns` and `tables`, but those are taken by `TableColumn` and `SqlaTable`. Originally I had the migration rename these tables, but it would force us to have downtime when upgrading, so I opted for using a prefix instead.
   
   I'm open to other suggestions for a prefix here. :)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r765312618



##########
File path: superset/migrations/versions/b8d3a24d9131_new_dataset_models.py
##########
@@ -0,0 +1,173 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""new dataset models
+
+Revision ID: b8d3a24d9131
+Revises: aea15018d53b
+Create Date: 2021-11-11 16:41:53.266965
+
+"""
+
+from uuid import uuid4
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy_utils import UUIDType
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable

Review comment:
       Ah, great point. Let me fix it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] betodealmeida commented on a change in pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17543:
URL: https://github.com/apache/superset/pull/17543#discussion_r773262400



##########
File path: superset/columns/schemas.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Schema for the column model.
+
+This model was introduced in SIP-68 (https://github.com/apache/superset/issues/14909),
+and represents a "column" in a table or dataset. In addition to a column, new models for
+tables, metrics, and datasets were also introduced.
+
+These models are not fully implemented, and shouldn't be used yet.
+"""
+
+from marshmallow_sqlalchemy import SQLAlchemyAutoSchema
+
+from superset.columns.models import Column
+
+
+class ColumnSchema(SQLAlchemyAutoSchema):

Review comment:
       This is used in this PR just for unit tests, I'll expand it into a manual schema later when we start reading from these models.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-actions[bot] commented on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-1020441531


   ⚠️ @betodealmeida Your base branch `master` has just also updated `superset/migrations`.
   
   ❗ **Please consider rebasing your branch to avoid db migration conflicts.**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] edited a comment on pull request #17543: feat: new dataset/table/column models

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17543:
URL: https://github.com/apache/superset/pull/17543#issuecomment-983787806


   # [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#17543](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (20def3e) into [master](https://codecov.io/gh/apache/superset/commit/7353a2bd75ae5c76458615443f530a688e78db1c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7353a2b) will **increase** coverage by `0.14%`.
   > The diff coverage is `98.34%`.
   
   > :exclamation: Current head 20def3e differs from pull request most recent head 77bd77d. Consider uploading reports for the commit 77bd77d to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/17543/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #17543      +/-   ##
   ==========================================
   + Coverage   76.81%   76.96%   +0.14%     
   ==========================================
     Files        1049     1067      +18     
     Lines       56653    57233     +580     
     Branches     7847     7847              
   ==========================================
   + Hits        43517    44048     +531     
   - Misses      12883    12932      +49     
     Partials      253      253              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `81.52% <98.34%> (?)` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `81.81% <98.34%> (?)` | |
   | python | `82.28% <98.34%> (+0.18%)` | :arrow_up: |
   | sqlite | `81.63% <98.34%> (-0.07%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.68% <97.33%> (+2.88%)` | :arrow_up: |
   | [superset/columns/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbW9kZWxzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/datasets/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvc2NoZW1hcy5weQ==) | `96.72% <100.00%> (+0.19%)` | :arrow_up: |
   | [superset/models/helpers.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | `90.83% <100.00%> (+0.25%)` | :arrow_up: |
   | [superset/security/manager.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc2VjdXJpdHkvbWFuYWdlci5weQ==) | `91.90% <100.00%> (+0.04%)` | :arrow_up: |
   | [superset/tables/models.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | |
   | [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | ... and [66 more](https://codecov.io/gh/apache/superset/pull/17543/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7353a2b...77bd77d](https://codecov.io/gh/apache/superset/pull/17543?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org