Skip to content

fix(admin): use ROWNUM for the Oracle instance identity lookup (#7724) - #7754

Merged
Aias00 merged 1 commit into
apache:masterfrom
konlue:fix/oracle-instance-selectone
Oct 18, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
konlue:fix/oracle-instance-selectone

Conversation

@konlue

@konlue konlue commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #7724.

Summary

InstanceInfoMapper.selectOneByQuery has no Oracle-specific mapping: the shared statement ends with LIMIT 1, which Oracle rejects. InstanceInfoServiceImpl#createOrUpdate calls this lookup before every insert/update, so instance identity persistence cannot work on an Oracle deployment.

The same mapper already demonstrates the supported pattern in its existed statement (databaseId="oracle" + ROWNUM = 1).

Modifications

  • Add a databaseId="oracle" variant of selectOneByQuery that keeps every identity/namespace predicate and ends with AND ROWNUM = 1.
  • Add a statement-shape regression test that parses the mapper with databaseId="oracle" and checks the resolved SQL.

Verifying this change

  • New InstanceInfoMapperOracleSqlTest: with databaseId="oracle", selectOneByQuery resolves to SQL containing ROWNUM and no LIMIT, with all four filters (instance_ip, namespace_id, instance_port, instance_type) intact.

  • Red-green: without the Oracle variant the test fails and reports the generic ... limit 1 statement; with it the test passes.

  • Targeted tests pass:

    ./mvnw -B -ntp -pl shenyu-admin test -Dtest=InstanceInfoMapperOracleSqlTest

    1/1 green.

  • Checkstyle passes:

    ./mvnw -B -ntp -pl shenyu-admin checkstyle:check

    clean.

Notes

AND ROWNUM = 1 applies after the filters and preserves the single-row semantics of LIMIT 1. The test pins MyBatis dialect selection locally; execution against a real Oracle instance is exercised by the repository's SQL matrix.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. The variant is a faithful copy of the generic statement plus a valid Oracle row limit.

Evidence

  • The five predicates (instance_ip, namespace_id, instance_port, instance_type, id) match the generic statement one for one — I diffed them, nothing was dropped and no bind name changed.
  • AND ROWNUM = 1 after the filters preserves LIMIT 1 semantics: ROWNUM is assigned to rows that already satisfy the WHERE clause, so the first matching row is returned. WHERE 1=1 keeps the dynamic predicates uniform.
  • The same pattern already exists in this mapper (existed, databaseId="oracle"), so the DatabaseIdProvider resolution is established, and the test pins which statement MyBatis picks.
  • The red-green is real: on master the resolved statement ends with LIMIT 1, so assertFalse(sql.contains("limit")) fails.
  • createOrUpdate builds its query from ip/port/type/namespace only, so the id filter is a bonus and its absence in the test does not weaken the regression.

Non-blocking

  1. Two statements now have to be kept in sync by hand. If you touch this again, a shared <sql> fragment for the predicate block with only the tail (LIMIT 1 vs AND ROWNUM = 1) differing would remove the drift risk.
  2. SElECT casing is inherited from the five existing statements in the file, so the new one is internally consistent — just noting that a case-sensitive SQL lint would flag all six.
  3. The test asserts four of the five filters; adding instanceId would cover the one predicate the caller never sets.

@Aias00
Aias00 merged commit 67ef2b4 into apache:master Oct 18, 2026
54 checks passed
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.

[BUG] Oracle instance status persistence uses an unsupported LIMIT clause

2 participants