Skip to content

fix(workflow-operator, v1.2): qualify the AsterixDB FROM clause; add AsterixDBSourceOpExec unit tests - #7006

Open
Yicong-Huang wants to merge 1 commit into
release/v1.2from
backport/6878-qualify-the-asterixdb-from-clause-add-as-v1.2
Open

fix(workflow-operator, v1.2): qualify the AsterixDB FROM clause; add AsterixDBSourceOpExec unit tests #7006
Yicong-Huang wants to merge 1 commit into
release/v1.2from
backport/6878-qualify-the-asterixdb-from-clause-add-as-v1.2

Conversation

@Yicong-Huang

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Backport of #6878 to release/v1.2, cherry-picked from 52584a8.

Source fix only. The test changes from #6878 were omitted: the touched test spec(s) do not exist on release/v1.2 (or depend on main-only test infrastructure), so backporting them cleanly is not possible. Only the source fix is carried over, per maintainer guidance.

Any related issues, documentation, discussions?

Backport of #6878. Originally linked #6875.

How was this PR tested?

Release-branch CI runs on this PR. The source change cherry-picked cleanly; test changes were intentionally dropped (see above).

Was this PR authored or co-authored using generative AI tooling?

Yes — backport prepared with Claude Code (mechanical cherry-pick + conflict resolution; the change itself is #6878 by its original author).

🤖 Generated with Claude Code

…xDBSourceOpExec unit tests (#6878)

### What changes were proposed in this PR?

**Source fix.** `AsterixDBSourceOpExec.addBaseSelect` built its query
with

```scala
s"... FROM $desc.database.$desc.table WHERE 1 = 1 "
```

In Scala that interpolates the `desc` **object** and then appends the
literal text `.database` / `.table`, so the generated FROM clause was
malformed and the operator's main query could never run. Line 288 of the
same file already does it correctly (`desc.database + "." +
desc.table`), confirming the intent. Fixed to
`${desc.database}.${desc.table}`.

The bug surfaced while writing tests for this file, which had **0%
coverage**. A reviewer rightly pointed out the first draft pinned the
buggy string; the spec now asserts the corrected clause instead.

**Tests.** Adds `AsterixDBSourceOpExecSpec` (47 tests) covering the
SQL++ string-building surface (no JDBC in this class):
- `addFilterConditions` and each branch it dispatches to —
`addKeywordSearch` (ftcontains; `IllegalArgumentException` on a
non-STRING column), `addRegexSearch`, `addGeoSearch` (create_rectangle
for 2 corners, create_polygon beyond that, OR-joined spatial_intersect,
`NumberFormatException` on a malformed point),
`addGeneralFilterCondition`;
- the skip paths for every toggle-off / missing-input case, and the
composed keyword→regex→geo→general ordering;
- schema derivation from the AsterixDB datatype, and the
base-select/limit/offset fragments.

**Test-infra fix.** `AsterixDBConnUtilSpec` now removes only its own
keys from the JVM-wide `AsterixDBConnUtil.asterixDBVersionMapping`
singleton instead of calling `clear()`: sbt runs suites in parallel
within one JVM, so a blanket clear could evict the new suite's cached
entry mid-run. Both suites also now call `super.beforeAll`/`beforeEach`
and wrap `afterAll` teardown in try/finally.

### Any related issues, documentation, discussions?

Closes #6875.

### How was this PR tested?

`sbt -java-home <jbr-17> "WorkflowOperator/testOnly *AsterixDB*"` -> 84
succeeded, 0 failed (across `AsterixDBSourceOpExecSpec`,
`AsterixDBConnUtilSpec`, `AsterixDBSourceOpDescSpec`).
`Test/scalafmtCheck`, `scalafmtCheck`, `Test/scalafix --check` and
`scalafix --check` all clean.

### Was this PR authored or co-authored using generative AI tooling?

(backported from commit 52584a8)

Generated-by: Claude Code (Opus 4.8 [1M context])
@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Jul 29, 2026
@Yicong-Huang
Yicong-Huang requested a review from xuang7 July 29, 2026 04:31
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@Yicong-Huang Yicong-Huang removed the release/v1.2 back porting to release/v1.2 label Jul 29, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 52.37%. Comparing base (fd5f487) to head (8c51b83).
⚠️ Report is 2 commits behind head on release/v1.2.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...r/source/sql/asterixdb/AsterixDBSourceOpExec.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##             release/v1.2    #7006      +/-   ##
==================================================
- Coverage           53.95%   52.37%   -1.58%     
- Complexity           1441     2487    +1046     
==================================================
  Files                 809     1077     +268     
  Lines               34144    42267    +8123     
  Branches             3448     4548    +1100     
==================================================
+ Hits                18421    22136    +3715     
- Misses              14815    18820    +4005     
- Partials              908     1311     +403     
Flag Coverage Δ *Carryforward flag
access-control-service 64.61% <ø> (ø)
agent-service 34.36% <ø> (ø) Carriedforward from fd5f487
amber 52.46% <0.00%> (-7.15%) ⬇️
computing-unit-managing-service 1.65% <ø> (ø)
config-service 56.06% <ø> (ø)
file-service 58.59% <ø> (ø)
frontend 47.19% <ø> (ø) Carriedforward from fd5f487
pyamber 90.87% <ø> (ø) Carriedforward from fd5f487
python 90.74% <ø> (ø) Carriedforward from fd5f487
workflow-compiling-service 58.69% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants