Skip to content

[SLT] Fix SLT code generation broken by the introduction of compiler-produced regions #6348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mihaibudiu merged 1 commit into from
May 30, 2026
Merged

[SLT] Fix SLT code generation broken by the introduction of compiler-produced regions #6348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...SQL-compiler/src/main/java/org/dbsp/sqlCompiler/compiler/backend/rust/RustFileWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public void write(DBSPCompiler compiler) {
// If it's a struct item, it is part of the list above
inner.accept(innerVisitor);
} else {
this.materializations.clearRegions();
DBSPCircuit outer = node.to(DBSPCircuit.class);
ToRustVisitor visitor = new ToRustVisitor(
compiler, this.builder(), outer.metadata, declarationsDone, this.materializations);
Expand Down
9 changes: 6 additions & 3 deletions sql-to-dbsp-compiler/SQL-compiler/src/main/java/org/dbsp/util/Utilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,9 @@ public static void runProcess(String directory, String... commands) throws IOExc
runProcess(directory, new HashMap<>(), commands);
}

/** Number of cargo jobs to use when testing locally */
static final int LOCAL_CARGO_JOBS = 6;

static final HashMap<String, String> STACK = new HashMap<>() {{ put("RUST_MIN_STACK", "8388608"); }};

static void compileAndTest(String directory, boolean quiet, String... extraArgs) throws IOException, InterruptedException {
Expand All @@ -456,7 +459,7 @@ static void compileAndTest(String directory, boolean quiet, String... extraArgs)
args.add("test");
if (!Utilities.inCI()) {
args.add("--jobs");
args.add("20");
args.add(Integer.toString(LOCAL_CARGO_JOBS));
}
args.addAll(Arrays.asList(extraArgs));
if (quiet) {
Expand Down Expand Up @@ -498,9 +501,9 @@ public static void compileAndCheckRust(String directory, boolean quiet, String..
List<String> args = new ArrayList<>();
args.add("cargo");
args.add("check");
if (Utilities.inCI()) {
if (!Utilities.inCI()) {

@mythical-fred mythical-fred May 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Worth a one-line comment on why compileAndCheckRust caps jobs only outside CI while compileAndTest does the same — symmetric now (good), but the original asymmetric version was almost certainly a typo nobody noticed. A short // Cap parallelism locally; CI controls -j via env next to both branches would prevent the next person from re-flipping it.

@mythical-fred mythical-fred May 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Behavior change here: the condition was if (Utilities.inCI()) and is now if (!Utilities.inCI()). Worth a sentence in the commit message — was the previous polarity the bug being fixed, or did this flip by accident along with the constant extraction?

args.add("--jobs");
args.add("6");
args.add(Integer.toString(LOCAL_CARGO_JOBS));
}
if (quiet)
args.add("--quiet");
Expand Down
14 changes: 14 additions & 0 deletions sql-to-dbsp-compiler/slt/out3147113901903481972.tmp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: unused variable: `pipeline_config`

@mythical-fred mythical-fred May 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks accidentally committed — stale cargo warning output with a local /home/mbudiu/ path baked in. Please drop and gitignore.

--> /home/mbudiu/git/feldera/crates/adapters/src/integrated.rs:97:5
|
97 | pipeline_config: &PipelineConfig,
| ^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_pipeline_config`
|
= note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

warning: unused variable: `runtime_env`
--> /home/mbudiu/git/feldera/crates/adapters/src/integrated.rs:98:5
|
98 | runtime_env: Arc<RuntimeEnv>,
| ^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_runtime_env`