Skip to content

Commit dba69ec

Browse files
cursoragenteluce2
andcommitted
Refactor: Do not implicitly add system columns to $select
Co-authored-by: eric.luce <eric.luce@proofgeist.com>
1 parent c09d7bb commit dba69ec

File tree

2 files changed

+34
-12
lines changed

2 files changed

+34
-12
lines changed

packages/fmodata/src/client/builders/query-string-builder.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,13 @@ export function buildSelectExpandQueryString(config: {
2424

2525
// Build $select
2626
if (config.selectedFields && config.selectedFields.length > 0) {
27-
// Add special columns if includeSpecialColumns is true and they're not already present
28-
let finalSelectedFields = [...config.selectedFields];
29-
if (config.includeSpecialColumns) {
30-
if (!finalSelectedFields.includes("ROWID")) {
31-
finalSelectedFields.push("ROWID");
32-
}
33-
if (!finalSelectedFields.includes("ROWMODID")) {
34-
finalSelectedFields.push("ROWMODID");
35-
}
36-
}
37-
27+
// Important: do NOT implicitly add system columns (ROWID/ROWMODID) here.
28+
// - `includeSpecialColumns` controls the Prefer header + response parsing, but should not
29+
// mutate/expand an explicit `$select` (e.g. when the user calls `.select({ ... })`).
30+
// - If system columns are desired with `.select()`, they must be explicitly included via
31+
// the `systemColumns` argument, which will already have added them to `selectedFields`.
3832
const selectString = formatSelectFields(
39-
finalSelectedFields,
33+
config.selectedFields,
4034
config.table,
4135
config.useEntityIds,
4236
);

packages/fmodata/tests/include-special-columns.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,34 @@ describe("includeSpecialColumns feature", () => {
433433
expect(firstRecord).not.toHaveProperty("ROWMODID");
434434
});
435435

436+
it("should not append ROWID/ROWMODID to explicit $select unless requested via systemColumns", () => {
437+
const db = connection.database("TestDB", {
438+
includeSpecialColumns: true,
439+
});
440+
441+
// Explicit select() should remain exact (no implicit system columns)
442+
const queryString = db
443+
.from(contactsTO)
444+
.list()
445+
.select({ name: contactsTO.name })
446+
.getQueryString();
447+
448+
expect(queryString).toContain("$select=");
449+
expect(queryString).toContain("name");
450+
expect(queryString).not.toContain("ROWID");
451+
expect(queryString).not.toContain("ROWMODID");
452+
453+
// But system columns should still be selectable when explicitly requested
454+
const queryStringWithSystemCols = db
455+
.from(contactsTO)
456+
.list()
457+
.select({ name: contactsTO.name }, { ROWID: true, ROWMODID: true })
458+
.getQueryString();
459+
460+
expect(queryStringWithSystemCols).toContain("ROWID");
461+
expect(queryStringWithSystemCols).toContain("ROWMODID");
462+
});
463+
436464
it("should work with single() method", async () => {
437465
const db = connection.database("TestDB", {
438466
includeSpecialColumns: true,

0 commit comments

Comments
 (0)