Skip to content

Commit bdd2f5f

Browse files
cursoragenteluce2
andcommitted
Refactor: Do not implicitly add system columns to $select
Co-authored-by: eric.luce <[email protected]>
1 parent c1b541c commit bdd2f5f

File tree

3 files changed

+36
-15
lines changed

3 files changed

+36
-15
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/src/client/update-builder.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type {
22
ExecutionContext,
33
ExecutableBuilder,
44
Result,
5-
WithSpecialColumns,
65
ExecuteOptions,
76
ExecuteMethodOptions,
87
} from "../types";

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,8 @@ describe("includeSpecialColumns feature", () => {
393393
includeSpecialColumns: true,
394394
});
395395

396-
// When select is called, special columns should not be included
397-
// (per OData spec, special columns are only included when no $select is used)
396+
// FileMaker OData requires ROWID/ROWMODID to be explicitly listed in $select
397+
// to be returned (they are only included when explicitly requested or when header is set and no $select is applied)
398398
let preferHeader: string | null = null;
399399
const { data } = await db
400400
.from(contactsTO)
@@ -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)