You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "zhutong6688 (via GitHub)" <gi...@apache.org> on 2025/02/19 14:52:31 UTC
[PR] Fix ODPS data source table preview failed #32301 [superset]
zhutong6688 opened a new pull request, #32310:
URL: https://github.com/apache/superset/pull/32310
### SUMMARY
I fixed an error when using sqllab to preview data after configuring the odps data source in superset, and added partition restrictions for previewing SQL
fix issue: https://github.com/apache/superset/issues/32301
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
This is a preview of the odps table data before the repair：
This is a preview of the repaired ODPS table data：
In addition, when using other data sources, there is no problem with testing and it does not affect other data sources
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. First, configure the ODPS data source
2. Create a new SQLlab page in the "SQL" column, select the database Schema
3. Select partitioned tables, non partitioned tables, and multi partitioned tables in sequence to test the correctness of data preview
4. Choose another data source, such as an example data source like SQLite, and follow the same steps as above to verify the validity of the data preview
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] 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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2680852927
> I'd recommend to have precommit installed and used to catch early CI issues.
Okay, got 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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1961885069
##########
superset/sql/parse.py:
##########
@@ -124,6 +124,21 @@ def __str__(self) -> str:
def __eq__(self, other: Any) -> bool:
return str(self) == str(other)
+@dataclass(eq=True, frozen=True)
+class Partition:
+ ispartitioned_table: bool
+ partition_column: list | None = None
+ def __str__(self) -> str:
+ """
+ Return the partition columns of table name.
Review Comment:
It'd be great if you have an example for stringified Partition object 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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2709270224
@hainenber @rusackas @sadpandajoe @bgreenlee Let's take a look at this PR (# 32556) later. The previous PR (# 32310) modified a lot of underlying basic classes. Although other data sources were not affected according to my testing, the testing unit classes of superset need to be adjusted too much. Therefore, I submitted a new PR, which tries not to modify the underlying basic classes as much as possible, but modifies the logical collection to the odps.by file, without causing too much impact on a large number of unit testing classes.
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2726208914
Let's pay attention to this PR（https://github.com/apache/superset/pull/32556） in the future. This PR will be closed for now. It has modified many underlying classes, which requires too many modifications to the test classes of the superset project itself。
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2698684578
I have adjusted the code. Thank you for agreeing to run the constantly blocked reviews
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2678843511
please help me run a CI again，thanks a lot
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962826938
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
+ if not database:
+ raise ValueError("Database not found")
+ uri = database.sqlalchemy_uri
+ passwd = database.password
+ pattern = re.compile(
+ r'odps://(?P<username>[^:]+):(?P<password>[^@]+)@(?P<project>[^/]+)/(?:\?endpoint=(?P<endpoint>[^&]+))'
Review Comment:
I mean there is no `match.group('password')` to indicate usage. Is the naming for the captured group only for readability?
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 closed pull request #32310: fix: ODPS data source table preview failed #32301
URL: https://github.com/apache/superset/pull/32310
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962042801
##########
superset/sql/parse.py:
##########
@@ -124,6 +124,21 @@ def __str__(self) -> str:
def __eq__(self, other: Any) -> bool:
return str(self) == str(other)
+@dataclass(eq=True, frozen=True)
+class Partition:
+ ispartitioned_table: bool
+ partition_column: list | None = None
+ def __str__(self) -> str:
+ """
+ Return the partition columns of table name.
Review Comment:
You're right, I'll add an example later
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2676709352
Please approve and agree a CI run again, Thanks a lot
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2675337000
approving a CI run, and adding a couple more reviewers for good measure :)
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1961861501
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
Review Comment:
can you add the function signature here?
##########
superset/databases/api.py:
##########
@@ -1002,15 +1003,16 @@ def table_metadata(self, pk: int) -> FlaskResponse:
parameters = QualifiedTableSchema().load(request.args)
except ValidationError as ex:
raise InvalidPayloadSchemaError(ex) from ex
-
- table = Table(parameters["name"], parameters["schema"], parameters["catalog"])
+ table_name = str(parameters["name"])
+ ispartitioned_table, partition_fields = DatabaseDAO.is_odps_partitioned_table(database,table_name)
Review Comment:
nit: `ispartitioned` is not snake-cased
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
+ if not database:
+ raise ValueError("Database not found")
+ uri = database.sqlalchemy_uri
+ passwd = database.password
+ pattern = re.compile(
+ r'odps://(?P<username>[^:]+):(?P<password>[^@]+)@(?P<project>[^/]+)/(?:\?endpoint=(?P<endpoint>[^&]+))'
Review Comment:
the named capture group `password` is unused here. Can be removed?
##########
superset/db_engine_specs/base.py:
##########
@@ -1689,7 +1690,11 @@ def select_star( # pylint: disable=too-many-arguments
full_table_name = cls.quote_table(table, engine.dialect)
qry = select(fields).select_from(text(full_table_name))
-
+ if database.backend == 'odps':
+ if partition.ispartitioned_table and partition.partition_column is not None and len(partition.partition_column) > 0:
Review Comment:
the `partition` variable can be `None` 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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1961879637
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
+ if not database:
+ raise ValueError("Database not found")
+ uri = database.sqlalchemy_uri
+ passwd = database.password
+ pattern = re.compile(
+ r'odps://(?P<username>[^:]+):(?P<password>[^@]+)@(?P<project>[^/]+)/(?:\?endpoint=(?P<endpoint>[^&]+))'
+ )
+ match = pattern.match(unquote(uri))
+ if match:
+ access_id = match.group('username')
+ project = match.group('project')
+ endpoint = match.group('endpoint')
+ access_key = passwd
Review Comment:
Re-assign DB's password to a 3rd variable seems odd 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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962048139
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
+ if not database:
+ raise ValueError("Database not found")
+ uri = database.sqlalchemy_uri
+ passwd = database.password
+ pattern = re.compile(
+ r'odps://(?P<username>[^:]+):(?P<password>[^@]+)@(?P<project>[^/]+)/(?:\?endpoint=(?P<endpoint>[^&]+))'
+ )
+ match = pattern.match(unquote(uri))
+ if match:
+ access_id = match.group('username')
+ project = match.group('project')
+ endpoint = match.group('endpoint')
+ access_key = passwd
Review Comment:
Access_key=passwd, I assigned passwd to Access_key, so that when using ODPS to build connections later, I have a visual experience of AK and Secret
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962826018
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
Review Comment:
it's the return value's type. Would be something similar to below
```
def is_odps_partitioned_table(cls, database: Database, table_name: str) -> Tuple(bool, [str]]:
```
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962835601
##########
superset/sql/parse.py:
##########
@@ -124,6 +124,21 @@ def __str__(self) -> str:
def __eq__(self, other: Any) -> bool:
return str(self) == str(other)
+@dataclass(eq=True, frozen=True)
+class Partition:
+ ispartitioned_table: bool
+ partition_column: list | None = None
+ def __str__(self) -> str:
+ """
+ Return the partition columns of table name.
Review Comment:
Oops, duplicate review. I'll resolve this one
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1961843257
##########
superset/db_engine_specs/base.py:
##########
@@ -1689,7 +1689,11 @@ def select_star( # pylint: disable=too-many-arguments
full_table_name = cls.quote_table(table, engine.dialect)
qry = select(fields).select_from(text(full_table_name))
-
+ if database.backend == 'odps':
+ if partition.ispartitioned_table and partition.partition_column is not None and len(partition.partition_column) > 0:
+ partition_str = partition.partition_column[0]
+ partition_str_where = f"CAST({partition_str} AS STRING) LIKE '%'"
+ qry = qry.where(text(partition_str_where))
Review Comment:
### Undefined partition variable in ODPS preview logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code attempts to use an undefined 'partition' variable which would cause a NameError when executing select_star for ODPS backend.
###### Why this matters
This will cause the preview functionality to fail for all ODPS tables, not just partitioned ones, preventing users from viewing any table data.
###### Suggested change ∙ *Feature Preview*
The partition parameter needs to be passed to the select_star method and used in the ODPS-specific logic. Add a 'partition' parameter to the method signature and use it in the ODPS condition:
```python
@classmethod
def select_star( # pylint: disable=too-many-arguments
cls,
database: Database,
table: Table,
engine: Engine,
limit: int = 100,
show_cols: bool = False,
indent: bool = True,
latest_partition: bool = True,
cols: list[ResultSetColumnType] | None = None,
partition: Partition | None = None,
) -> str:
...
if database.backend == 'odps' and partition:
if partition.ispartitioned_table and partition.partition_column is not None and len(partition.partition_column) > 0:
partition_str = partition.partition_column[0]
partition_str_where = f"CAST({partition_str} AS STRING) LIKE '%'"
qry = qry.where(text(partition_str_where))
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a13e9991-debe-485f-9de9-4ff0001c4edd?suggestedFixEnabled=true)
💬 Chat with Korbit by mentioning @korbit-ai.
</sub>
<!--- korbi internal id:78a81700-ab28-49bc-9852-442b7e06ed77 -->
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2668958632
Thank you for the PR! I left a few comments and also you'll need to run added changes against established pre-commit hooks [here](https://superset.apache.org/docs/contributing/development/#git-hooks).
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962043973
##########
superset/db_engine_specs/base.py:
##########
@@ -1689,7 +1690,11 @@ def select_star( # pylint: disable=too-many-arguments
full_table_name = cls.quote_table(table, engine.dialect)
qry = select(fields).select_from(text(full_table_name))
-
+ if database.backend == 'odps':
+ if partition.ispartitioned_table and partition.partition_column is not None and len(partition.partition_column) > 0:
Review Comment:
I will add a non null judgment later
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962040210
##########
superset/databases/api.py:
##########
@@ -1002,15 +1003,16 @@ def table_metadata(self, pk: int) -> FlaskResponse:
parameters = QualifiedTableSchema().load(request.args)
except ValidationError as ex:
raise InvalidPayloadSchemaError(ex) from ex
-
- table = Table(parameters["name"], parameters["schema"], parameters["catalog"])
+ table_name = str(parameters["name"])
+ ispartitioned_table, partition_fields = DatabaseDAO.is_odps_partitioned_table(database,table_name)
Review Comment:
Ispartitioned is for the convenience of determining whether the current table is partitioned, and also for the convenience of further expanding new functions in the future
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962033514
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
+ if not database:
+ raise ValueError("Database not found")
+ uri = database.sqlalchemy_uri
+ passwd = database.password
+ pattern = re.compile(
+ r'odps://(?P<username>[^:]+):(?P<password>[^@]+)@(?P<project>[^/]+)/(?:\?endpoint=(?P<endpoint>[^&]+))'
Review Comment:
Take a look at the back, this one is in use
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962835058
##########
superset/databases/api.py:
##########
@@ -1002,15 +1003,16 @@ def table_metadata(self, pk: int) -> FlaskResponse:
parameters = QualifiedTableSchema().load(request.args)
except ValidationError as ex:
raise InvalidPayloadSchemaError(ex) from ex
-
- table = Table(parameters["name"], parameters["schema"], parameters["catalog"])
+ table_name = str(parameters["name"])
+ ispartitioned_table, partition_fields = DatabaseDAO.is_odps_partitioned_table(database,table_name)
Review Comment:
I mean, `ispartitioned` doesn't follow usual Python snake-case naming convention. It usually goes like `is_partitioned_table`, fyi
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2693639959
@hainenber please help me run a CI again，thanks a lot
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2680190680
I'd recommend to have precommit installed and used to catch early CI issues.
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2680187904
@hainenber May I ask if this error is caused by not installing and using pre commit?
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2695068687
I found that the code for table preview has been greatly adjusted, which is exactly where I fixed it. The interface calls are different from what I modified before, and the only thing fixed is the improvement. It is suggested to slow down the improvement of this section of code, as there are indeed many bugs and bug fixes cannot keep up with the speed of improving the code. This is not reasonable, and it is more appropriate to prioritize bug fixes in the code.
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2676756587
Could you please help me run a CI again so that I can see if there are any other check errors，Thanks again
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2676713862
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/32310?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `40.42553%` with `28 lines` in your changes missing coverage. Please review.
> Project coverage is 76.17%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`98583c9`)](https://app.codecov.io/gh/apache/superset/commit/98583c939dc69a5ab433d6879194f9009201b99c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1476 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/32310?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/daos/database.py](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&filepath=superset%2Fdaos%2Fdatabase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGFvcy9kYXRhYmFzZS5weQ==) | 26.08% | [17 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/databases/api.py](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&filepath=superset%2Fdatabases%2Fapi.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | 16.66% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/db\_engine\_specs/base.py](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&filepath=superset%2Fdb_engine_specs%2Fbase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | 33.33% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [superset/sql/parse.py](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&filepath=superset%2Fsql%2Fparse.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc3FsL3BhcnNlLnB5) | 75.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/32310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #32310 +/- ##
===========================================
+ Coverage 60.48% 76.17% +15.68%
===========================================
Files 1931 546 -1385
Lines 76236 39138 -37098
Branches 8568 0 -8568
===========================================
- Hits 46114 29814 -16300
+ Misses 28017 9324 -18693
+ Partials 2105 0 -2105
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `48.45% <38.29%> (-0.71%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.89% <40.42%> (?)` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.17% <40.42%> (+12.68%)` | :arrow_up: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.40% <40.42%> (?)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/32310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/32310?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2679172474
please help me run a CI again，thanks a lot
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2676175724
Please approve and agree a CI run，I have made the necessary corrections based on the error message
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962838802
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
+ if not database:
+ raise ValueError("Database not found")
+ uri = database.sqlalchemy_uri
+ passwd = database.password
+ pattern = re.compile(
+ r'odps://(?P<username>[^:]+):(?P<password>[^@]+)@(?P<project>[^/]+)/(?:\?endpoint=(?P<endpoint>[^&]+))'
+ )
+ match = pattern.match(unquote(uri))
+ if match:
+ access_id = match.group('username')
+ project = match.group('project')
+ endpoint = match.group('endpoint')
+ access_key = passwd
Review Comment:
Are there any benefits aside of readability? I'm not holding strong opinion on this but feels like this assignment is a bit excessive.
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2676183541
Some of the data source test classes did not pass. Let me take a look at the problem again and see if there are any issues with the test classes being written
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1969946340
##########
requirements/base.in:
##########
@@ -23,3 +23,4 @@ numexpr>=2.9.0
# 5.0.0 has a sensitive deprecation used in other libs
# -> https://github.com/aio-libs/async-timeout/blob/master/CHANGES.rst#500-2024-10-31
async_timeout>=4.0.0,<5.0.0
+pyodps>=0.12.2
Review Comment:
I don't see this library getting used in the PR. Can you provide some contexts?
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2693927665
Could you please help me agree and check the code online? Thank you
--
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
Re: [PR] fix: ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #32310:
URL: https://github.com/apache/superset/pull/32310#issuecomment-2694047580
CI has already run. I’ll be reviewing your code.
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962042457
##########
superset/sql/parse.py:
##########
@@ -124,6 +124,21 @@ def __str__(self) -> str:
def __eq__(self, other: Any) -> bool:
return str(self) == str(other)
+@dataclass(eq=True, frozen=True)
+class Partition:
+ ispartitioned_table: bool
+ partition_column: list | None = None
+ def __str__(self) -> str:
+ """
+ Return the partition columns of table name.
Review Comment:
You're right, I'll add an example later
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "zhutong6688 (via GitHub)" <gi...@apache.org>.
zhutong6688 commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1962037767
##########
superset/daos/database.py:
##########
@@ -165,7 +168,30 @@ def get_ssh_tunnel(cls, database_id: int) -> SSHTunnel | None:
)
return ssh_tunnel
-
+
+ @classmethod
+ def is_odps_partitioned_table(cls, database: Database, table_name: str):
Review Comment:
Are you saying adding some function descriptions or comments?
--
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
Re: [PR] Fix ODPS data source table preview failed #32301 [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on code in PR #32310:
URL: https://github.com/apache/superset/pull/32310#discussion_r1961884705
##########
superset/sql/parse.py:
##########
@@ -124,6 +124,21 @@ def __str__(self) -> str:
def __eq__(self, other: Any) -> bool:
return str(self) == str(other)
+@dataclass(eq=True, frozen=True)
+class Partition:
+ ispartitioned_table: bool
+ partition_column: list | None = None
+ def __str__(self) -> str:
+ """
+ Return the partition columns of table name.
Review Comment:
It'd be great if you have an example for stringified Partition object 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