yashmayya opened a new pull request, #18623: URL: https://github.com/apache/pinot/pull/18623
## Problem `mvn clean install` intermittently fails to build `pinot-common` with: ``` [ERROR] .../sql/parsers/CalciteSqlParser.java:[69,42] cannot find symbol [ERROR] symbol: class SqlParserImpl [ERROR] location: package org.apache.pinot.sql.parsers.parser ``` The same failure shows up "randomly" inside IDEs. ### Root cause `pinot-common` generates its SQL parser in two `generate-sources` stages: 1. **FMPP** injects Pinot's custom grammar (`config.fmpp` + `templates/Parser.jj`) into Calcite's parser template and writes `target/generated-sources/javacc/Parser.jj`. 2. **javacc-maven-plugin** compiles that grammar into `SqlParserImpl.java`. Stage 1 lived **only inside the `sqlparser` profile**, which auto-activated via `<file><missing>.../javacc/Parser.jj</missing></file>`. The catch: **Maven evaluates profile activation once, at reactor start — before `clean` runs.** So in a single `mvn clean install` after any prior build: - Reactor start: the previous build's `Parser.jj` still exists → `sqlparser` is **inactive** → FMPP is excluded from the plan. - `clean` deletes `target/` → `Parser.jj` is gone. - `generate-sources`: FMPP never runs; javacc finds no grammar → generates nothing. - `compile`: `SqlParserImpl` was never generated → **cannot find symbol**. It "works sometimes" because a fresh checkout (no `target/`) activates the profile, and `mvn clean` followed by a **separate** `mvn install` also works. IDEs snapshot active profiles at import time (while `Parser.jj` exists) and then their rebuild wipes it — hence the random IDE failures. The old profile's own comment documented this footgun but only offered manual workarounds (`-Psqlparser` or two separate invocations). ## Fix - Remove the `sqlparser` profile. - Move the FMPP step into the main `<build><plugins>`, declared **before** `javacc-maven-plugin`, so it **always** runs in `generate-sources` (same-phase goals execute in POM declaration order). - Preserve the incremental-build speed the profile gate was protecting: FMPP now writes to a staging dir (`target/generated-sources/fmpp`), then an Ant `<copy>` with a `<different ignoreFileTimes="true">` selector promotes `Parser.jj` into the javacc source dir **only when the generated grammar content actually changed**. Unchanged rebuilds don't bump the timestamp, so javacc and the compiler skip — no spurious full-module recompiles. ## Rationale (alternatives considered) - **Always-run FMPP without the staging/`<different>` trick** — simplest, but reintroduces the always-recompile cost the profile was created to avoid (FMPP rewrites its output every run). - **Keep the profile, change its activation trigger** — file-based activation that `clean` can touch risks the same ordering trap; property-based activation re-creates the `-Psqlparser` opt-in ergonomics problem. - **Switch to a dedicated `fmpp-maven-plugin`** (Calcite/Drill upstream style) — clean, but adds a new plugin to the build's governance/supply-chain surface for what is fundamentally a build-ordering bug. - **Chosen: staging dir + `<different>` selector** — reuses the already-present `maven-antrun-plugin` + `fmpp` dependency (no new plugin), guarantees the grammar is regenerated after a `clean` in every invocation mode, and keeps incremental builds fast. As a bonus, IDE codegen is now deterministic. ## Test plan - [x] Reproduced the original failure: single-invocation `clean test-compile` with `Parser.jj` present → `cannot find symbol: SqlParserImpl`. - [x] After the fix, the same single-invocation `clean install` succeeds and `SqlParserImpl` is generated. - [x] Incremental no-op: rebuilding without grammar changes leaves `Parser.jj` untouched; javacc reports "all parsers are up to date" and the compiler skips. - [x] Change detection: corrupting the on-disk `Parser.jj` triggers the copy and restores the correct grammar + regenerates `SqlParserImpl`. - [x] Full module build (`clean install`, build cache disabled): `BUILD SUCCESS`, `Tests run: 1449, Failures: 0, Errors: 0`, shade/license/rat/spotless all clean, artifacts installed. -- 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]
