codeant-ai-for-open-source[bot] commented on code in PR #39509:
URL: https://github.com/apache/superset/pull/39509#discussion_r3559520352
##########
superset/db_engine_specs/elasticsearch.py:
##########
@@ -136,6 +231,30 @@ class ElasticSearchEngineSpec(BaseEngineSpec): # pylint:
disable=abstract-metho
type_code_map: dict[int, str] = {} # loaded from get_datatype only if
needed
+ SQL_ENDPOINT = "/_sql"
Review Comment:
**Suggestion:** Add explicit type annotations for the newly introduced
class-level endpoint constants to satisfy the type-hint requirement for
relevant variables. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The custom rule requires type hints for relevant variables that can be
annotated. These new class-level constants are plain string assignments without
annotations, so the suggestion correctly identifies a real type-hint omission.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1e21e78d44a44c01826a4046030ad26b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1e21e78d44a44c01826a4046030ad26b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/elasticsearch.py
**Line:** 233:234
**Comment:**
*Custom Rule: Add explicit type annotations for the newly introduced
class-level endpoint constants to satisfy the type-hint requirement for
relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=effd365546992f8532d9a128586d459a537549fe9f151f8ae5c8d218c30c735e&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=effd365546992f8532d9a128586d459a537549fe9f151f8ae5c8d218c30c735e&reaction=dislike'>π</a>
##########
superset/db_engine_specs/elasticsearch.py:
##########
@@ -204,6 +324,30 @@ class OpenDistroEngineSpec(BaseEngineSpec): # pylint:
disable=abstract-method
engine = "odelasticsearch"
engine_name = "OpenSearch (OpenDistro)"
+ SQL_ENDPOINT = "/_opendistro/_sql"
+ SQL_CLOSE_ENDPOINT = "/_opendistro/_sql/close"
Review Comment:
**Suggestion:** Add explicit type annotations for the newly introduced
OpenDistro endpoint constants so these relevant class variables are
type-hinted. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The custom rule requires type hints for relevant variables that can be
annotated. These newly introduced class constants are unannotated string
variables, so this is a genuine violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ba73d3668b3d4d3e909ca88dc24908f0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ba73d3668b3d4d3e909ca88dc24908f0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/elasticsearch.py
**Line:** 327:328
**Comment:**
*Custom Rule: Add explicit type annotations for the newly introduced
OpenDistro endpoint constants so these relevant class variables are type-hinted.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=68ab7b4c410e0046f99e1aa70fb98c1dc3466267364d06ea099b1f211187ea81&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=68ab7b4c410e0046f99e1aa70fb98c1dc3466267364d06ea099b1f211187ea81&reaction=dislike'>π</a>
##########
superset/views/datasource/utils.py:
##########
@@ -176,15 +178,88 @@ def get_samples( # pylint: disable=too-many-arguments
if count_star_data.get("status") == QueryStatus.FAILED:
raise DatasetSamplesFailedError(count_star_data.get("error"))
- sample_data = samples_instance.get_payload()["queries"][0]
+ engine_spec = datasource.database.db_engine_spec
Review Comment:
**Suggestion:** Add an explicit local type annotation for this newly
introduced variable to satisfy the projectβs type-hinting requirement for
relevant variables. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This is newly added Python code and the local variable `engine_spec` is
introduced without an explicit type annotation, which matches the project rule
requiring type hints for relevant new or modified Python variables when they
can be annotated.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=009b1ee022434d23a3c7f77bccb537aa&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=009b1ee022434d23a3c7f77bccb537aa&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/datasource/utils.py
**Line:** 181:181
**Comment:**
*Custom Rule: Add an explicit local type annotation for this newly
introduced variable to satisfy the projectβs type-hinting requirement for
relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=3bdbaaf17cdc3364fc48545128879ea5f49fac092e40b869b55d92b81ba3f1ed&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=3bdbaaf17cdc3364fc48545128879ea5f49fac092e40b869b55d92b81ba3f1ed&reaction=dislike'>π</a>
##########
superset/views/datasource/utils.py:
##########
@@ -176,15 +178,88 @@ def get_samples( # pylint: disable=too-many-arguments
if count_star_data.get("status") == QueryStatus.FAILED:
raise DatasetSamplesFailedError(count_star_data.get("error"))
- sample_data = samples_instance.get_payload()["queries"][0]
+ engine_spec = datasource.database.db_engine_spec
+ row_offset = limit_clause["row_offset"]
+ row_limit = limit_clause["row_limit"]
- if sample_data.get("status") == QueryStatus.FAILED:
- QueryCacheManager.delete(count_star_data.get("cache_key"),
CacheRegion.DATA)
- raise DatasetSamplesFailedError(sample_data.get("error"))
+ if not engine_spec.supports_offset and row_offset > 0:
+ try:
+ sample_data = _fetch_samples_via_cursor(
+ datasource=datasource,
+ samples_instance=samples_instance,
+ count_star_data=count_star_data,
+ page_index=row_offset // row_limit,
+ page_size=row_limit,
+ )
+ except DatasetSamplesFailedError:
+ raise
+ except Exception as exc:
+ QueryCacheManager.delete(
+ count_star_data.get("cache_key"), CacheRegion.DATA
+ )
+ logger.exception("Cursor-based samples pagination failed")
+ raise DatasetSamplesFailedError(
+ "Failed to fetch samples via cursor pagination"
+ ) from exc
+ else:
+ sample_data = samples_instance.get_payload()["queries"][0]
+ if sample_data.get("status") == QueryStatus.FAILED:
+ QueryCacheManager.delete(
+ count_star_data.get("cache_key"), CacheRegion.DATA
+ )
+ raise DatasetSamplesFailedError(sample_data.get("error") or "")
sample_data["page"] = page
sample_data["per_page"] = per_page
sample_data["total_count"] = count_star_data["data"][0]["COUNT(*)"]
return sample_data
except (IndexError, KeyError) as exc:
raise DatasetSamplesFailedError from exc
+
+
+def _fetch_samples_via_cursor(
+ datasource: Datasource,
+ samples_instance: QueryContext,
+ count_star_data: dict[str, Any],
+ page_index: int,
+ page_size: int,
+) -> dict[str, Any]:
+ """
+ Fetch a single page of samples via engine-spec cursor pagination.
+
+ Used when ``datasource.database.db_engine_spec.supports_offset`` is
+ False and a non-first page is requested. Compiles the same SQL Superset
+ would run for the normal samples payload β without executing it β and
+ delegates cursor iteration to the engine spec. The engine spec is
+ responsible for stripping any trailing ``LIMIT`` from the SQL so the
+ cursor is not capped to a single page.
+
+ ``coltypes`` are inferred from the returned rows with
+ ``extract_dataframe_dtypes``, the same function the non-cursor path
+ uses to type page 1 β it works off the actual returned values, not
+ ``cursor.description``, so no ES-type-to-coltype translator is needed
+ and no extra query is required to source them.
+ """
+ query_obj = samples_instance.queries[0]
Review Comment:
**Suggestion:** Add an explicit local type annotation for this newly
introduced variable in the helper function to comply with required type hints
on relevant variables. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
This is newly added Python code and the local variable `query_obj` is
introduced without an explicit type annotation, which is a real instance of the
repositoryβs type-hinting rule for new or modified annotatable variables.
</details>
<details>
<summary><b>Rule source π </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0e34d38f274145ada6ce2945344f45ce&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0e34d38f274145ada6ce2945344f45ce&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/datasource/utils.py
**Line:** 243:243
**Comment:**
*Custom Rule: Add an explicit local type annotation for this newly
introduced variable in the helper function to comply with required type hints
on relevant variables.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=4236d67d3cb688cd6eada838ebeac8f6a0b8f16090698ae922447805d30255ba&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39509&comment_hash=4236d67d3cb688cd6eada838ebeac8f6a0b8f16090698ae922447805d30255ba&reaction=dislike'>π</a>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
