Skip to content

feat(cli): add starrocks support#671

Open
ido177 wants to merge 1 commit intogetnao:mainfrom
ido177:add-starrocks
Open

feat(cli): add starrocks support#671
ido177 wants to merge 1 commit intogetnao:mainfrom
ido177:add-starrocks

Conversation

@ido177
Copy link
Copy Markdown

@ido177 ido177 commented Apr 24, 2026

Hi
This PR introduces support for StarRocks by adding a dedicated connector and extending catalog handling.

  1. StarRocks connector
    The existing MySQL connector is not compatible with StarRocks in our use case.
    When used via Ibis, it triggers errors due to SHOW statements being executed inside explicit transactions:
Detail message: Explicit transaction only support 
begin/commit/rollback/insert/update/delete/set/select statements.

Since StarRocks does not allow SHOW statements within transactions, this leads to failures.
To resolve this, we switched to using mysql-connector-python for interacting with StarRocks, avoiding the problematic behavior.

  1. Multi-catalog support
    Added support for working with multiple catalogs in StarRocks.
    Unlike MySQL, StarRocks has a hierarchical structure:
    catalog → database → table
    The previous implementation assumed a single-catalog model (as in MySQL), which is not sufficient for StarRocks. This change enables specifying and working with multiple catalogs.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 24, 2026

🚀 Preview Deployment

URL https://pr-671-02209e9.preview.getnao.io
Commit 02209e9

⚠️ No LLM API keys configured - you'll see the API key setup flow when trying to chat.


Preview will be automatically removed when this PR is closed.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cli/nao_core/config/databases/starrocks.py">

<violation number="1" location="cli/nao_core/config/databases/starrocks.py:68">
P1: SQL identifiers and literals are interpolated without quoting/escaping, so valid StarRocks names can fail and user-controlled values can alter the query.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread cli/nao_core/config/databases/starrocks.py
@ido177 ido177 marked this pull request as draft April 24, 2026 09:33
@ido177 ido177 marked this pull request as ready for review April 24, 2026 11:58
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cli/nao_core/config/databases/starrocks.py">

<violation number="1" location="cli/nao_core/config/databases/starrocks.py:151">
P2: `SHOW FULL COLUMNS` nullability is read from the collation column instead of the `Null` column, so fallback column metadata can report incorrect nullable flags.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread cli/nao_core/config/databases/starrocks.py
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 12 files

@Bl3f
Copy link
Copy Markdown
Contributor

Bl3f commented Apr 30, 2026

Hello, @ido177 thank you so much for your first contribution. As I don't have any Starrocks available I can't try, just to confirm did you try with a running Starrocks?

Also, there is some uv issues + ruff installs, can you find a way to fix it?

@ido177 ido177 force-pushed the add-starrocks branch 2 times, most recently from 9d9445a to 4911ec6 Compare April 30, 2026 16:31
@ido177
Copy link
Copy Markdown
Author

ido177 commented Apr 30, 2026

Hi @Bl3f

yes, I've tested this on my local environment and it works fine

just updated the branch and fixed 1 line, make lint says that All checks passed!

@ido177
Copy link
Copy Markdown
Author

ido177 commented Apr 30, 2026

Also, there is some uv issues + ruff installs, can you find a way to fix it?

Can you provide errors? I can't find the log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants