feat: Add Oracle storage adapter #9998
Conversation
Introduces a new Oracle storage adapter, including a client implementation, configuration parser, and query formatter to support pg-promise style queries. Adds a set of Oracle SQL utility functions for array and JSON operations, and an index to load these SQL files. This enables Oracle as a backend option with feature parity for array and JSON manipulation.
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds a new Oracle storage adapter: an oracledb-backed client with pg-promise-like API, URI parser, query formatter, Oracle-specific SQL functions, SQL loader, and an OracleStorageAdapter implementing schema, queries, transactions, and lifecycle integration. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Adapter as OracleStorageAdapter
participant Client as OracleClient
participant Parser as OracleConfigParser
participant Formatter as QueryFormatter
participant Pool as Connection Pool
participant Oracle as oracledb
participant DB as Oracle DB
rect `#dcebf6`
Note over Adapter,Client: Initialization
Adapter->>Client: createClient(uri, dbOptions)
Client->>Parser: getDatabaseOptionsFromURI(uri)
Parser-->>Client: parsed config
Client->>Oracle: createPool(poolConfig)
Oracle-->>Client: pool
Client-->>Adapter: client (with $pool)
end
rect `#e6f9e6`
Note over Adapter,DB: Query flow (task/any)
Adapter->>Client: task/any(query, params)
Client->>Pool: acquire connection
Pool->>Oracle: getConnection
Oracle-->>Client: connection
Client->>Formatter: formatQuery(query, params)
Formatter-->>Client: sql + binds
Client->>Oracle: execute(sql, binds, options)
Oracle->>DB: run SQL
DB-->>Oracle: rows
Oracle-->>Client: result
Client->>Pool: release connection
Client-->>Adapter: results
end
rect `#fff0d9`
Note over Adapter,DB: Transaction flow (tx)
Adapter->>Client: tx(callback)
Client->>Pool: acquire connection
Oracle-->>Client: connection
Client->>Adapter: invoke tx callback(context)
Adapter->>Client: one/any/... (within tx)
Client->>Oracle: execute (within tx)
alt success
Client->>Oracle: COMMIT
else error
Client->>Oracle: ROLLBACK
end
Client->>Pool: release connection
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (4)
src/Adapters/Storage/Oracle/sql/array/contains.sql (1)
10-18: Consider using EXISTS for early termination.The current implementation counts all matching rows before checking if
cnt >= 1. UsingEXISTSat the top level would short-circuit on the first match, improving performance for large arrays.🔎 Proposed optimization
- SELECT COUNT(*) - INTO cnt - FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) arr - WHERE EXISTS ( - SELECT 1 - FROM JSON_TABLE("values", '$[*]' COLUMNS (val JSON PATH '$')) vals - WHERE arr.value = vals.val - ); - RETURN CASE WHEN cnt >= 1 THEN 1 ELSE 0 END; + SELECT CASE WHEN EXISTS ( + SELECT 1 + FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) arr + WHERE EXISTS ( + SELECT 1 + FROM JSON_TABLE("values", '$[*]' COLUMNS (val JSON PATH '$')) vals + WHERE arr.value = vals.val + ) + ) THEN 1 ELSE 0 END + INTO cnt + FROM DUAL; + RETURN cnt;src/Adapters/Storage/Oracle/sql/array/add-unique.sql (1)
12-21: Redundant DISTINCT wrapper around UNION.
UNIONalready eliminates duplicate rows between both inputs. The outerSELECT DISTINCTis redundant and adds unnecessary processing.🔎 Proposed simplification
SELECT JSON_ARRAYAGG(value ORDER BY value) INTO result FROM ( - SELECT DISTINCT value - FROM ( - SELECT value - FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) - UNION - SELECT value - FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) - ) + SELECT value + FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) + UNION + SELECT value + FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) );src/Adapters/Storage/Oracle/sql/index.js (2)
3-4: Useconstinstead ofvar.Modern JavaScript style prefers
constfor variables that aren't reassigned.-var path = require('path'); -var fs = require('fs'); +const path = require('path'); +const fs = require('fs');
7-10: Consider adding error handling for missing SQL files.
fs.readFileSyncwill throw if a file is missing, which could cause cryptic startup failures. Adding a try-catch with a descriptive error message would improve debuggability.function sql(file) { - var fullPath = path.join(__dirname, file); - return fs.readFileSync(fullPath, 'utf8'); + const fullPath = path.join(__dirname, file); + try { + return fs.readFileSync(fullPath, 'utf8'); + } catch (err) { + throw new Error(`Failed to load Oracle SQL file: ${file} - ${err.message}`); + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/Adapters/Storage/Oracle/OracleClient.jssrc/Adapters/Storage/Oracle/OracleConfigParser.jssrc/Adapters/Storage/Oracle/QueryFormatter.jssrc/Adapters/Storage/Oracle/sql/array/add-unique.sqlsrc/Adapters/Storage/Oracle/sql/array/add.sqlsrc/Adapters/Storage/Oracle/sql/array/contains-all-regex.sqlsrc/Adapters/Storage/Oracle/sql/array/contains-all.sqlsrc/Adapters/Storage/Oracle/sql/array/contains.sqlsrc/Adapters/Storage/Oracle/sql/array/remove.sqlsrc/Adapters/Storage/Oracle/sql/index.jssrc/Adapters/Storage/Oracle/sql/misc/json-object-set-keys.sql
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
src/Adapters/Storage/Oracle/OracleConfigParser.js
🧬 Code graph analysis (3)
src/Adapters/Storage/Oracle/QueryFormatter.js (1)
src/Adapters/AdapterLoader.js (1)
module(49-49)
src/Adapters/Storage/Oracle/OracleClient.js (1)
src/Adapters/Storage/Oracle/OracleConfigParser.js (1)
databaseOptions(3-3)
src/Adapters/Storage/Oracle/OracleConfigParser.js (2)
src/Adapters/Storage/Oracle/OracleClient.js (1)
require(3-3)src/Adapters/AdapterLoader.js (1)
module(49-49)
🔇 Additional comments (5)
src/Adapters/Storage/Oracle/sql/array/remove.sql (1)
1-23: LGTM - Clean implementation of array element removal.The logic correctly filters out elements present in the
valuesarray usingNOT EXISTS, and theCOALESCEensures a deterministic empty array return.Consider adding explicit NULL handling for input parameters if Oracle doesn't handle them gracefully (e.g.,
IF "array" IS NULL THEN RETURN JSON_ARRAY(); END IF;), though this may be intentionally delegated to the caller.src/Adapters/Storage/Oracle/sql/array/contains-all-regex.sql (1)
14-17: Consider returning 1 for empty values array.Returning 0 when
values_len = 0may be counterintuitive. In set theory, "contains all of nothing" (vacuous truth) is typically true. This aligns with how$all: []behaves in MongoDB (matches everything).Verify this matches the expected Parse Server semantics.
src/Adapters/Storage/Oracle/sql/misc/json-object-set-keys.sql (1)
1-23: LGTM - Clean implementation using JSON_MERGEPATCH.The function correctly uses
JSON_MERGEPATCHfor upserting a key. Minor note: the filename uses plural "keys" while the function name is singular "key" - consider aligning them for consistency.Also note that
JSON_MERGEPATCHwith a NULL first argument may behave unexpectedly in Oracle; verify this matches expected behavior if NULL input is possible.src/Adapters/Storage/Oracle/sql/index.js (1)
17-17: Thearray/contains-all.sqlfile exists in the repository at the expected location and is properly referenced.src/Adapters/Storage/Oracle/OracleClient.js (1)
33-48: Fix Oracle connection string format to match oracledb Easy Connect requirements.The connection string construction has critical format issues:
The SID fallback (
host:port:sid) is invalid for oracledb. Easy Connect does not support SID format. SID requires a full connect descriptor:(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=host)(PORT=port))(CONNECT_DATA=(SID=sid)))The fallback to just
hostis incomplete and will fail. It must include port and a service name or SID.When
serviceNameis not provided, the code should not buildhost:port/without a valid service name. Either requireserviceNameexplicitly or use a proper full connect descriptor for SID.Consider revising the logic to explicitly handle two cases: (a) service name via Easy Connect (
host:port/serviceName), or (b) SID via full connect descriptor if needed, rather than the invalidhost:port:sidformat.
| return { query, params: params || [] }; | ||
| } | ||
|
|
||
| export function createClient(uri, databaseOptions) { |
There was a problem hiding this comment.
Inconsistent module format: ES6 export mixed with CommonJS require.
Line 13 uses ES6 export function while lines 1-3 use CommonJS require. This mixing can cause issues depending on the module system in use. Parse Server uses CommonJS throughout.
🔎 Proposed fix
-export function createClient(uri, databaseOptions) {
+function createClient(uri, databaseOptions) {
// ... existing code ...
return { client, oracledb };
}
+
+module.exports = { createClient };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function createClient(uri, databaseOptions) { | |
| function createClient(uri, databaseOptions) { | |
| // ... existing code ... | |
| return { client, oracledb }; | |
| } | |
| module.exports = { createClient }; |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around line 13, there is a mixed
module format: CommonJS require is used at the top of the file while line 13
exports with ES6 `export function`. Replace the ES6 export with a CommonJS
export (attach the function to module.exports or exports) so the file
consistently uses CommonJS; ensure any other identifiers exported in this file
follow the same CommonJS pattern and update references/imports elsewhere if
necessary.
| if (options && options.direct) { | ||
| // Direct connection (for schema notifications) | ||
| return await oracledb.getConnection(poolConfig); | ||
| } | ||
| return await pool.getConnection(); |
There was a problem hiding this comment.
Direct connection bypasses pool but still uses poolConfig.
When options.direct is true (line 58), the code calls oracledb.getConnection(poolConfig) which creates a standalone connection but with pool configuration parameters. This is misleading and could cause unexpected behavior since pool-specific settings (poolMin, poolMax, etc.) don't apply to standalone connections.
🔎 Proposed fix
if (options && options.direct) {
// Direct connection (for schema notifications)
- return await oracledb.getConnection(poolConfig);
+ const { user, password, connectString, externalAuth } = poolConfig;
+ return await oracledb.getConnection({ user, password, connectString, externalAuth });
}🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around lines 58 to 62, the
direct-connection branch incorrectly passes poolConfig to oracledb.getConnection
(which contains pool-specific keys like poolMin/poolMax); change it to build and
pass a connectionConfig containing only connection-level properties (e.g., user,
password, connectString and any non-pool options) or accept a dedicated
connectionOptions parameter, and then call
oracledb.getConnection(connectionConfig) so standalone connections never receive
pool settings.
| async task(taskName, callback) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const taskContext = { | ||
| any: async (query, params) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| return result.rows || []; | ||
| }, | ||
| one: async (query, params, transform) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| maxRows: 1, | ||
| }); | ||
| const row = result.rows && result.rows[0]; | ||
| return transform ? transform(row) : row; | ||
| }, | ||
| none: async (query, params) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| }, | ||
| map: async (query, params, transform) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| return (result.rows || []).map(transform); | ||
| }, | ||
| tx: async (txName, callback) => { | ||
| return await callback(taskContext); | ||
| }, | ||
| batch: async (promises) => { | ||
| return await Promise.all(promises); | ||
| }, | ||
| }; | ||
| return await callback(taskContext); | ||
| } finally { | ||
| await conn.close(); | ||
| } | ||
| }, | ||
| async any(query, params) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| return result.rows || []; | ||
| } finally { | ||
| await conn.close(); | ||
| } | ||
| }, | ||
| async one(query, params, transform) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| maxRows: 1, | ||
| }); | ||
| const row = result.rows && result.rows[0]; | ||
| return transform ? transform(row) : row; | ||
| } finally { | ||
| await conn.close(); | ||
| } | ||
| }, | ||
| async none(query, params) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| } finally { | ||
| await conn.close(); | ||
| } | ||
| }, | ||
| async map(query, params, transform) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| return (result.rows || []).map(transform); | ||
| } finally { | ||
| await conn.close(); | ||
| } | ||
| }, | ||
| async tx(txName, callback) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const txContext = { | ||
| any: async (query, params) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| return result.rows || []; | ||
| }, | ||
| one: async (query, params, transform) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| maxRows: 1, | ||
| }); | ||
| const row = result.rows && result.rows[0]; | ||
| return transform ? transform(row) : row; | ||
| }, | ||
| none: async (query, params) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| }, | ||
| map: async (query, params, transform) => { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| const result = await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| return (result.rows || []).map(transform); | ||
| }, | ||
| batch: async (promises) => { | ||
| return await Promise.all(promises); | ||
| }, | ||
| ctx: { duration: 0 }, | ||
| }; | ||
| const result = await callback(txContext); | ||
| await conn.commit(); | ||
| return result; | ||
| } catch (error) { | ||
| await conn.rollback().catch(() => {}); | ||
| throw error; | ||
| } finally { | ||
| await conn.close(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication between task, tx, and standalone methods.
The taskContext (lines 68-103), txContext (lines 162-195), and standalone methods (lines 109-157) contain nearly identical implementations of any, one, none, and map. This violates DRY principles and makes maintenance difficult.
🔎 Refactoring approach
Extract the common query execution logic into shared helper functions:
function createQueryContext(conn) {
return {
any: async (query, params) => {
const formatted = formatQueryForOracle(query, params);
const result = await conn.execute(formatted.query, formatted.params, {
outFormat: oracledb.OUT_FORMAT_OBJECT,
});
return result.rows || [];
},
one: async (query, params, transform) => {
const formatted = formatQueryForOracle(query, params);
const result = await conn.execute(formatted.query, formatted.params, {
outFormat: oracledb.OUT_FORMAT_OBJECT,
maxRows: 1,
});
const row = result.rows && result.rows[0];
return transform ? transform(row) : row;
},
none: async (query, params) => {
const formatted = formatQueryForOracle(query, params);
await conn.execute(formatted.query, formatted.params, {
outFormat: oracledb.OUT_FORMAT_OBJECT,
});
},
map: async (query, params, transform) => {
const formatted = formatQueryForOracle(query, params);
const result = await conn.execute(formatted.query, formatted.params, {
outFormat: oracledb.OUT_FORMAT_OBJECT,
});
return (result.rows || []).map(transform);
},
batch: async (promises) => {
return await Promise.all(promises);
},
};
}Then reuse this in task, tx, and standalone methods.
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around lines 64-205 there is
duplicated implementations of any/one/none/map/batch across taskContext,
txContext and the standalone methods; extract a shared factory (e.g.
createQueryContext(conn)) that returns any/one/none/map/batch implemented once
using formatQueryForOracle and conn.execute, then replace the duplicated blocks
by calling that factory for task, for the standalone methods (wrap connect/close
around calls to the shared context) and for tx (create the shared context from
conn, then attach tx-specific fields like ctx.duration and keep the existing
commit/rollback handling). Ensure no behavioral changes: preserve
outFormat/maxRows options, return values, batch using Promise.all, and
transaction commit/rollback/close semantics.
| tx: async (txName, callback) => { | ||
| return await callback(taskContext); | ||
| }, |
There was a problem hiding this comment.
Nested transaction doesn't actually start a transaction.
The tx method inside taskContext (lines 97-99) just calls the callback with taskContext instead of txContext. This means nested transactions won't have proper commit/rollback behavior, which could lead to data consistency issues.
🔎 Proposed fix
The nested tx should either:
- Delegate to the outer
client.txmethod to start a real transaction - Document that nested transactions are not supported and throw an error
- Implement savepoint-based nested transactions
tx: async (txName, callback) => {
- return await callback(taskContext);
+ // Option 1: Use outer tx method
+ return await client.tx(txName, callback);
+ // OR Option 2: Explicitly disallow
+ throw new Error('Nested transactions are not supported. Use client.tx() at the top level.');
},Committable suggestion skipped: line range outside the PR's diff.
| } catch (error) { | ||
| await conn.rollback().catch(() => {}); | ||
| throw error; |
There was a problem hiding this comment.
Silent error swallowing during rollback masks failures.
Line 200 catches and ignores all rollback errors with .catch(() => {}). If the rollback fails (e.g., connection lost), this error is hidden, making debugging difficult and potentially leaving the database in an inconsistent state.
🔎 Proposed fix
} catch (error) {
- await conn.rollback().catch(() => {});
+ try {
+ await conn.rollback();
+ } catch (rollbackError) {
+ // Log but don't throw to preserve original error
+ console.error('Rollback failed:', rollbackError);
+ }
throw error;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| await conn.rollback().catch(() => {}); | |
| throw error; | |
| } catch (error) { | |
| try { | |
| await conn.rollback(); | |
| } catch (rollbackError) { | |
| // Log but don't throw to preserve original error | |
| console.error('Rollback failed:', rollbackError); | |
| } | |
| throw error; |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around lines 199 to 201, the
catch block currently swallows rollback errors via .catch(() => {}); instead,
capture rollback failures and surface them: when conn.rollback() fails, log the
rollback error (using the module logger or console.error) and do not silently
ignore it — either attach it to the original error (e.g., add rollbackError
property) or rethrow a combined error after logging so callers can see both the
original and rollback failures; ensure the original error is still thrown after
handling/logging the rollback failure to preserve the original failure context.
| matches.sort((a, b) => b.position - a.position); | ||
|
|
||
| for (const m of matches) { | ||
| const paramValue = params[m.index - 1]; | ||
| let replacement; | ||
|
|
||
| if (m.modifier === 'name') { | ||
| replacement = `"${paramValue}"`; | ||
| } else if (m.modifier === 'raw') { | ||
| replacement = paramValue; | ||
| } else { | ||
| const paramName = `param${paramIndex}`; | ||
| replacement = `:${paramName}`; | ||
| oracleParams[paramName] = paramValue; | ||
| paramIndex++; | ||
| } | ||
|
|
||
| formattedQuery = | ||
| formattedQuery.substring(0, m.position) + | ||
| replacement + | ||
| formattedQuery.substring(m.position + m.full.length); | ||
| } |
There was a problem hiding this comment.
Same parameter ordering bug exists here.
The formatQueryWithNamedParams function has the identical issue—matches are iterated in reverse position order while paramIndex increments sequentially, causing parameter name misalignment with their values.
Apply the same two-pass fix: assign param names in original query order first, then replace strings in reverse order.
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/QueryFormatter.js around lines 84 to 105, the
loop assigns param names while replacing placeholders in a single pass which
misaligns paramIndex because replacements are done in reverse position order;
fix by doing two passes: first iterate matches sorted by position ascending to
assign param names (create paramN names, populate oracleParams[paramN] with
values and store the replacement string or paramName on each match), increment
paramIndex in that pass only; second, iterate matches sorted by position
descending to perform string replacements using the precomputed replacement
values (handling 'name' and 'raw' modifiers at replacement time), so
replacements do not change subsequent match positions and parameter values
remain aligned.
| FROM ( | ||
| SELECT DISTINCT value | ||
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) | ||
| UNION ALL | ||
| SELECT value | ||
| FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) | ||
| ); |
There was a problem hiding this comment.
Clarify: Inconsistent duplicate handling between inputs.
The query applies DISTINCT to the first array but uses UNION ALL to append the second array. This means:
- Duplicates within
"array"are collapsed - All elements from
"values"are kept (including duplicates) - Elements appearing in both arrays will be duplicated in the result
If the intent is simple concatenation, remove DISTINCT. If the intent is to produce unique elements, use UNION instead of UNION ALL:
🔎 Option A: Simple concatenation (allow duplicates)
FROM (
- SELECT DISTINCT value
+ SELECT value
FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$'))
UNION ALL
SELECT value
FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$'))
);🔎 Option B: Unique elements only (use UNION)
FROM (
- SELECT DISTINCT value
- FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$'))
- UNION ALL
+ SELECT value
+ FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$'))
+ UNION
SELECT value
FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$'))
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM ( | |
| SELECT DISTINCT value | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) | |
| UNION ALL | |
| SELECT value | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) | |
| ); | |
| FROM ( | |
| SELECT value | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) | |
| UNION ALL | |
| SELECT value | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) | |
| ); |
| FROM ( | |
| SELECT DISTINCT value | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) | |
| UNION ALL | |
| SELECT value | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) | |
| ); | |
| FROM ( | |
| SELECT value | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value JSON PATH '$')) | |
| UNION | |
| SELECT value | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (value JSON PATH '$')) | |
| ); |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/sql/array/add.sql around lines 12 to 18, the
query applies DISTINCT to the first JSON input but uses UNION ALL for the
second, causing inconsistent duplicate handling; decide intended behavior and
make both sides consistent: for simple concatenation (allow duplicates) remove
DISTINCT so both inputs are taken as-is, or for unique results replace UNION ALL
with UNION so duplicates across and within inputs are removed; update the SQL
accordingly and ensure tests reflect the chosen behavior.
| SELECT COUNT(DISTINCT arr.value) | ||
| INTO match_cnt | ||
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr | ||
| WHERE EXISTS ( | ||
| SELECT 1 | ||
| FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals | ||
| WHERE REGEXP_LIKE(arr.value, vals.val) | ||
| ); | ||
|
|
||
| RETURN CASE WHEN match_cnt = values_len THEN 1 ELSE 0 END; |
There was a problem hiding this comment.
Critical: Logic counts matching array elements instead of matched regexes.
The current implementation counts how many distinct array elements match any regex, then checks if that equals the number of regexes. This produces incorrect results.
Example failure case:
array = ["abc", "abd"],values = ["a.*", "d.*"]- Both "abc" and "abd" match "a.*", so
match_cnt = 2 values_len = 2, so function returns 1 (true)- Bug: "d.*" doesn't match any element, so result should be 0 (false)
The correct approach is to count how many regexes have at least one matching array element.
🔎 Proposed fix
- SELECT COUNT(DISTINCT arr.value)
+ SELECT COUNT(DISTINCT vals.val)
INTO match_cnt
- FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr
+ FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals
WHERE EXISTS (
SELECT 1
- FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals
- WHERE REGEXP_LIKE(arr.value, vals.val)
+ FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr
+ WHERE REGEXP_LIKE(arr.value, vals.val)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SELECT COUNT(DISTINCT arr.value) | |
| INTO match_cnt | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr | |
| WHERE EXISTS ( | |
| SELECT 1 | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals | |
| WHERE REGEXP_LIKE(arr.value, vals.val) | |
| ); | |
| RETURN CASE WHEN match_cnt = values_len THEN 1 ELSE 0 END; | |
| SELECT COUNT(DISTINCT vals.val) | |
| INTO match_cnt | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals | |
| WHERE EXISTS ( | |
| SELECT 1 | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr | |
| WHERE REGEXP_LIKE(arr.value, vals.val) | |
| ); | |
| RETURN CASE WHEN match_cnt = values_len THEN 1 ELSE 0 END; |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/sql/array/contains-all-regex.sql around lines 19
to 28, the query currently counts distinct array elements that match any regex
(producing false positives); change the logic to count how many distinct regexes
have at least one matching array element instead. Replace the current
aggregation to select/count DISTINCT vals.val (from JSON_TABLE("values",...))
where an EXISTS subquery checks for any arr.value matching that vals.val via
REGEXP_LIKE, store that count in match_cnt, and then compare match_cnt to
values_len; ensure DISTINCT is applied to the regex values and the EXISTS
subquery iterates the array JSON table.
| IF values_len = 0 THEN | ||
| RETURN 0; | ||
| END IF; |
There was a problem hiding this comment.
Incorrect logic for empty values array.
When values_len = 0 (empty array), the function returns 0 (false). However, by the principle of vacuous truth, an array contains all elements of an empty set, so this should return 1 (true). This logic error could break queries that check containment against empty arrays.
🔎 Proposed fix
- IF values_len = 0 THEN
- RETURN 0;
- END IF;
+ IF values_len = 0 THEN
+ RETURN 1;
+ END IF;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| IF values_len = 0 THEN | |
| RETURN 0; | |
| END IF; | |
| IF values_len = 0 THEN | |
| RETURN 1; | |
| END IF; |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/sql/array/contains-all.sql around lines 16 to 18,
the function currently returns 0 when values_len = 0 which is incorrect; change
the behavior to return 1 for an empty values array (vacuous truth). Update the
conditional to return true (1) when values_len = 0 and leave the rest of the
logic unchanged so queries checking containment against empty arrays succeed.
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr | ||
| WHERE EXISTS ( | ||
| SELECT 1 | ||
| FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals | ||
| WHERE arr.value = vals.val |
There was a problem hiding this comment.
VARCHAR2(4000) size limit may truncate values and cause incorrect matches.
Using VARCHAR2(4000) as the column type in JSON_TABLE will truncate any array values exceeding 4000 bytes. Truncated values could match incorrectly (e.g., two different long strings truncated to the same 4000-byte prefix would be treated as equal), leading to false positives in containment checks.
Consider using CLOB or VARCHAR2(32767) (the maximum for PL/SQL) to handle larger values:
🔎 Proposed fix
SELECT COUNT(DISTINCT arr.value)
INTO match_cnt
- FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr
+ FROM JSON_TABLE("array", '$[*]' COLUMNS (value CLOB PATH '$')) arr
WHERE EXISTS (
SELECT 1
- FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals
+ FROM JSON_TABLE("values", '$[*]' COLUMNS (val CLOB PATH '$')) vals
WHERE arr.value = vals.val
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value VARCHAR2(4000) PATH '$')) arr | |
| WHERE EXISTS ( | |
| SELECT 1 | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (val VARCHAR2(4000) PATH '$')) vals | |
| WHERE arr.value = vals.val | |
| FROM JSON_TABLE("array", '$[*]' COLUMNS (value CLOB PATH '$')) arr | |
| WHERE EXISTS ( | |
| SELECT 1 | |
| FROM JSON_TABLE("values", '$[*]' COLUMNS (val CLOB PATH '$')) vals | |
| WHERE arr.value = vals.val |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/sql/array/contains-all.sql around lines 22-26,
the JSON_TABLE columns are declared as VARCHAR2(4000) which can truncate values
>4000 bytes and cause false-positive matches; change the column types to a
larger type (e.g., CLOB or VARCHAR2(32767)) in both JSON_TABLE calls and ensure
any comparisons or functions accept CLOBs (use TO_CLOB or DBMS_LOB.COMPARE where
necessary) so long values are preserved and equality checks remain correct.
…lmohib/parse-server into feature/oracle-23ai-adapter
Introduces OracleStorageAdapter and related utilities for supporting Oracle as a storage backend. Includes schema translation, query building, and Oracle-specific SQL handling to enable Parse Server compatibility with Oracle databases.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (3)
27-31: Minor: Redundant use ofargumentsobject.Since
argsalready contains all arguments via rest parameters, usingarguments[0]is redundant and less clear. Consider usingargs[0]directly.🔎 Proposed fix
const debug = function (...args: any) { - args = ['ORACLE: ' + arguments[0]].concat(args.slice(1, args.length)); + args = ['ORACLE: ' + args[0]].concat(args.slice(1, args.length)); const log = logger.getLogger(); log.debug.apply(log, args); };
504-504: Useconstinstead ofvar.Modern JavaScript best practice is to use
const(orletif reassignment is needed) instead ofvarto avoid hoisting issues and improve code clarity.🔎 Proposed fix
- var createConstraint = (baseArray, notIn) => { + const createConstraint = (baseArray, notIn) => {
1047-1051: Consider usingvarforGeoPointCoderto avoid TDZ.Since
GeoPointCoderis used inbuildWhereClause(line 699) before its declaration here, usingvarinstead of object literal assignment would allow hoisting. However, the cleaner fix is to move this declaration earlier in the file (before line 308).See the earlier comment about the TDZ violation for the recommended fix.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.
Applied to files:
src/Adapters/Storage/Oracle/OracleStorageAdapter.js
🧬 Code graph analysis (1)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (2)
src/Adapters/Storage/Oracle/OracleClient.js (1)
oracledb(2-2)src/Controllers/DatabaseController.js (1)
classExists(1204-1204)
🪛 Biome (2.1.2)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js
[error] 11-11: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 27-27: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 291-296: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 308-308: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 997-997: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1012-1012: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1014-1014: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1025-1025: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1053-1053: classes can only implement interfaces in TypeScript files
(parse)
[error] 1054-1054: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1055-1055: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1058-1058: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1059-1059: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1060-1060: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1061-1061: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1062-1062: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1063-1063: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1064-1064: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1064-1064: expected a semicolon to end the class property, but found none
(parse)
[error] 1064-1064: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '?'.
Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.
(parse)
[error] 1065-1065: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1067-1067: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1086-1086: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1086-1086: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1091-1091: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1091-1091: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1128-1128: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1132-1132: expected , but instead found _SCHEMA
Remove _SCHEMA
(parse)
[error] 1132-1132: expected , but instead found "'"
Remove "'"
(parse)
[error] 1149-1149: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1156-1156: expected } but instead the file ends
the file ends here
(parse)
[error] 371-371: This number literal will lose precision at runtime.
The value at runtime will be 9223372036854776000
(lint/correctness/noPrecisionLoss)
[error] 699-699: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (5)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (5)
66-174: LGTM!The comparator mappings, value transformation functions, and schema helpers are well-structured and follow the same patterns as other storage adapters in the codebase.
190-248: LGTM!The dot field handling correctly transforms Parse-style dot notation into Oracle JSON path syntax using
JSON_VALUE. The recursive object building inhandleDotFieldsproperly handles the__op: 'Delete'case.
931-1045: LGTM!The regex processing functions properly handle various regex patterns including
\Q...\Eliteral escaping, whitespace removal for extended mode, and Unicode character classes. These appear to be correctly adapted from the Postgres adapter.
1-12: Regarding static analysis warnings about type annotations.The Biome tool flags Flow type annotations as TypeScript-only syntax errors. Since Parse Server uses Flow (indicated by
// @flow), these warnings are false positives and can be ignored. The project's build tooling (Babel) handles Flow syntax stripping.
1100-1109: Code correctly implements pool shutdown with proper interface bridging.The
OracleClientwrapper properly exposes the$pool.end()interface (matching pg-promise conventions) which internally callspool.close()(the oracledb standard method). The implementation is correct and requires no changes.
| default: | ||
| throw `no type for ${JSON.stringify(type)} yet`; | ||
| } |
There was a problem hiding this comment.
Throw an Error object instead of a string literal.
Throwing a plain string loses the stack trace and makes debugging harder. Throw a Parse.Error or standard Error for consistency with the rest of the codebase.
🔎 Proposed fix
default:
- throw `no type for ${JSON.stringify(type)} yet`;
+ throw new Parse.Error(
+ Parse.Error.INTERNAL_SERVER_ERROR,
+ `no type for ${JSON.stringify(type)} yet`
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| default: | |
| throw `no type for ${JSON.stringify(type)} yet`; | |
| } | |
| default: | |
| throw new Parse.Error( | |
| Parse.Error.INTERNAL_SERVER_ERROR, | |
| `no type for ${JSON.stringify(type)} yet` | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js around lines 61-63, the
code throws a raw string (`throw \`no type for ${JSON.stringify(type)} yet\``);
replace this with throwing an Error instance (e.g., `throw new Error(\`no type
for ${JSON.stringify(type)} yet\`)`) or the codebase-standard Parse.Error (e.g.,
`throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, \`no type for
${JSON.stringify(type)} yet\`)`), and add any required import for Parse when
using Parse.Error; this preserves stack traces and matches project error
handling conventions.
| if (schema.fields[fieldName] && schema.fields[fieldName].type === 'Number') { | ||
| // Should always return zero results | ||
| const MAX_INT_PLUS_ONE = 9223372036854775808; | ||
| values.push(fieldName, MAX_INT_PLUS_ONE); | ||
| } else { | ||
| values.push(fieldName, fieldValue ? 1 : 0); | ||
| } | ||
| index += 2; |
There was a problem hiding this comment.
Precision loss with large integer constant.
9223372036854775808 exceeds JavaScript's Number.MAX_SAFE_INTEGER (2^53 - 1). At runtime, this will be coerced to 9223372036854776000, potentially causing unexpected query behavior.
🔎 Proposed fix using BigInt or string
// Should always return zero results
- const MAX_INT_PLUS_ONE = 9223372036854775808;
- values.push(fieldName, MAX_INT_PLUS_ONE);
+ // Use a string representation that Oracle can handle as a large number
+ const MAX_INT_PLUS_ONE = '9223372036854775808';
+ values.push(fieldName, MAX_INT_PLUS_ONE);Alternatively, if the Oracle driver supports BigInt:
- const MAX_INT_PLUS_ONE = 9223372036854775808;
+ const MAX_INT_PLUS_ONE = 9223372036854775808n;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (schema.fields[fieldName] && schema.fields[fieldName].type === 'Number') { | |
| // Should always return zero results | |
| const MAX_INT_PLUS_ONE = 9223372036854775808; | |
| values.push(fieldName, MAX_INT_PLUS_ONE); | |
| } else { | |
| values.push(fieldName, fieldValue ? 1 : 0); | |
| } | |
| index += 2; | |
| if (schema.fields[fieldName] && schema.fields[fieldName].type === 'Number') { | |
| // Should always return zero results | |
| // Use a string representation that Oracle can handle as a large number | |
| const MAX_INT_PLUS_ONE = '9223372036854775808'; | |
| values.push(fieldName, MAX_INT_PLUS_ONE); | |
| } else { | |
| values.push(fieldName, fieldValue ? 1 : 0); | |
| } | |
| index += 2; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 371-371: This number literal will lose precision at runtime.
The value at runtime will be 9223372036854776000
(lint/correctness/noPrecisionLoss)
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js around lines 369 to 376,
the large integer literal 9223372036854775808 exceeds Number.MAX_SAFE_INTEGER
and will suffer precision loss; replace it with a safe representation
(preferably the string "9223372036854775808" or a BigInt literal
9223372036854775808n if the Oracle driver supports BigInt) and ensure the value
is bound to the query using the correct driver binding/type for Oracle NUMBER
values so the DB receives the exact integer; update the push so
values.push(fieldName, <safe-representation>) and adjust any binding/config
nearby to treat that parameter as a numeric/string as required by the driver.
| if (fieldValue.$ne !== undefined) { | ||
| if (isArrayField) { | ||
| fieldValue.$ne = JSON.stringify([fieldValue.$ne]); | ||
| patterns.push(`array_contains($${index}:name, $${index + 1}) = 0`); | ||
| } else { | ||
| if (fieldValue.$ne === null) { | ||
| patterns.push(`$${index}:name IS NOT NULL`); | ||
| values.push(fieldName); | ||
| index += 1; | ||
| continue; | ||
| } else { | ||
| // if not null, we need to manually exclude null | ||
| if (fieldValue.$ne.__type === 'GeoPoint') { | ||
| // Oracle Spatial - simplified for now | ||
| patterns.push( | ||
| `($${index}:name IS NULL OR SDO_GEOM.SDO_DISTANCE($${index}:name, SDO_GEOMETRY(2001, 4326, SDO_POINT_TYPE($${index + 1}, $${index + 2}, NULL), NULL, NULL), 0.005) > 0.001)` | ||
| ); | ||
| } else { | ||
| if (fieldName.indexOf('.') >= 0) { | ||
| const castType = toOracleValueCastType(fieldValue.$ne); | ||
| const constraintFieldName = castType | ||
| ? `CAST ((${transformDotField(fieldName)}) AS ${castType})` | ||
| : transformDotField(fieldName); | ||
| patterns.push( | ||
| `(${constraintFieldName} <> $${index + 1} OR ${constraintFieldName} IS NULL)` | ||
| ); | ||
| } else if (typeof fieldValue.$ne === 'object' && fieldValue.$ne.$relativeTime) { | ||
| throw new Parse.Error( | ||
| Parse.Error.INVALID_JSON, | ||
| '$relativeTime can only be used with the $lt, $lte, $gt, and $gte operators' | ||
| ); | ||
| } else { | ||
| patterns.push(`($${index}:name <> $${index + 1} OR $${index}:name IS NULL)`); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| if (fieldValue.$ne.__type === 'GeoPoint') { | ||
| const point = fieldValue.$ne; | ||
| values.push(fieldName, point.longitude, point.latitude); | ||
| index += 3; | ||
| } else { | ||
| // TODO: support arrays | ||
| values.push(fieldName, fieldValue.$ne); | ||
| index += 2; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing control flow after array field handling for $ne.
When isArrayField is true (lines 406-408), the code sets fieldValue.$ne to a JSON string and pushes a pattern, but then continues execution to line 442 which checks fieldValue.$ne.__type. Since $ne is now a string, .___type is undefined, causing the code to fall through to lines 446-450 where it incorrectly pushes duplicate values. This likely needs a continue or return after line 408.
🔎 Proposed fix
if (isArrayField) {
fieldValue.$ne = JSON.stringify([fieldValue.$ne]);
patterns.push(`array_contains($${index}:name, $${index + 1}) = 0`);
+ values.push(fieldName, fieldValue.$ne);
+ index += 2;
} else {Then remove the duplicate handling at lines 442-450, or add proper guard logic.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js around lines 405-451,
when isArrayField is true the code JSON.stringifys fieldValue.$ne and pushes an
array_contains pattern but then falls through to the later GeoPoint/values
handling (lines ~442-450), causing incorrect duplicate value pushes; add an
immediate continue after pushing the array_contains pattern and ensure you push
the correct values and increment index for the array case (fieldName and the
JSON string) before continuing, and remove or guard the later duplicate handling
(or explicitly skip it when typeof fieldValue.$ne === 'string') so array $ne
processing does not execute the non-array branch.
| createExplainableQuery(query: string, analyze: boolean = false) { | ||
| // Oracle uses EXPLAIN PLAN | ||
| if (analyze) { | ||
| return 'EXPLAIN PLAN FOR ' + query; | ||
| } else { | ||
| return 'EXPLAIN PLAN FOR ' + query; | ||
| } | ||
| } |
There was a problem hiding this comment.
The analyze parameter is not being used.
Both branches of the conditional return the same string. In Oracle, EXPLAIN PLAN FOR only generates the execution plan without running the query. If you need actual runtime statistics, you'd need a different approach (e.g., tracing, DBMS_XPLAN.DISPLAY_CURSOR).
🔎 Proposed fix - either remove unused parameter or implement properly
Option 1 - Remove unused parameter:
- createExplainableQuery(query: string, analyze: boolean = false) {
- // Oracle uses EXPLAIN PLAN
- if (analyze) {
- return 'EXPLAIN PLAN FOR ' + query;
- } else {
- return 'EXPLAIN PLAN FOR ' + query;
- }
+ createExplainableQuery(query: string) {
+ // Oracle uses EXPLAIN PLAN (actual execution statistics require different approach)
+ return 'EXPLAIN PLAN FOR ' + query;
}Option 2 - Document the limitation:
createExplainableQuery(query: string, analyze: boolean = false) {
// Oracle uses EXPLAIN PLAN
- if (analyze) {
- return 'EXPLAIN PLAN FOR ' + query;
- } else {
- return 'EXPLAIN PLAN FOR ' + query;
- }
+ // Note: Oracle's EXPLAIN PLAN does not execute the query.
+ // The 'analyze' parameter is accepted for API compatibility but has no effect.
+ return 'EXPLAIN PLAN FOR ' + query;
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 1091-1091: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1091-1091: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js around lines 1091 to
1098, the analyze parameter is unused because both branches return the same
'EXPLAIN PLAN FOR ' + query string; either remove the unused analyze parameter
from the method signature and callers, or implement correct behavior for
analyze=true by returning/throwing a clear note that runtime statistics require
a different approach (e.g., use DBMS_XPLAN.DISPLAY_CURSOR after executing the
query and capturing cursor IDs or provide a separate method that runs the query
and calls DBMS_XPLAN.DISPLAY_CURSOR), and update JSDoc/comments to document the
limitation and expected workflow.
Introduces OracleStorageAdapter with full schema and object management for Oracle DB. Updates Controllers to support 'oracle:' URIs, enabling Parse Server to use Oracle as a backend database option.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (7)
1090-1098: Unusedanalyzeparameter.Both branches return the same string. Oracle's
EXPLAIN PLAN FORonly generates the execution plan without running the query. Theanalyzeparameter has no effect.🔎 Proposed fix
- //Note that analyze=true will run the query, executing INSERTS, DELETES, etc. - createExplainableQuery(query: string, analyze: boolean = false) { - // Oracle uses EXPLAIN PLAN - if (analyze) { - return 'EXPLAIN PLAN FOR ' + query; - } else { - return 'EXPLAIN PLAN FOR ' + query; - } + // Oracle uses EXPLAIN PLAN (does not execute the query) + createExplainableQuery(query: string) { + return 'EXPLAIN PLAN FOR ' + query; }
61-63: Throw an Error object instead of a string.Throwing a plain string loses the stack trace. The codebase standard is to use
Parse.Errorfor consistency.🔎 Proposed fix
default: - throw `no type for ${JSON.stringify(type)} yet`; + throw new Parse.Error( + Parse.Error.INTERNAL_SERVER_ERROR, + `no type for ${JSON.stringify(type)} yet` + );
369-376: Precision loss with large integer constant.The literal
9223372036854775808exceedsNumber.MAX_SAFE_INTEGER(2^53 - 1) and will be coerced to9223372036854776000at runtime, causing incorrect query behavior.🔎 Proposed fix
// Should always return zero results - const MAX_INT_PLUS_ONE = 9223372036854775808; - values.push(fieldName, MAX_INT_PLUS_ONE); + const MAX_INT_PLUS_ONE = '9223372036854775808'; + values.push(fieldName, MAX_INT_PLUS_ONE);
697-704: Temporal Dead Zone violation:GeoPointCoderused before declaration.
GeoPointCoderis referenced here but declared at line 1047. This causes aReferenceErrorat runtime due to JavaScript's Temporal Dead Zone.🔎 Proposed fix
Move the
GeoPointCoderdeclaration (lines 1047-1051) to before thebuildWhereClausefunction, around line 307:+// Helper for GeoPoint validation +const GeoPointCoder = { + isValidJSON(value) { + return typeof value === 'object' && value !== null && value.__type === 'GeoPoint'; + }, +}; + const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClause => {
405-451: Control flow issue: array$nehandling falls through.When
isArrayFieldis true (lines 406-408), the code setsfieldValue.$neto a JSON string and pushes a pattern, but execution continues to line 442 which checksfieldValue.$ne.__type. Since$neis now a string,.__typeis undefined, causing the code to fall through to lines 446-450 and incorrectly push duplicate values.🔎 Proposed fix
if (fieldValue.$ne !== undefined) { if (isArrayField) { fieldValue.$ne = JSON.stringify([fieldValue.$ne]); patterns.push(`array_contains($${index}:name, $${index + 1}) = 0`); + values.push(fieldName, fieldValue.$ne); + index += 2; + continue; } else {This ensures the array case exits before reaching the non-array handling logic.
1128-1147: SQL syntax error in table existence check.Line 1132 has malformed SQL with incorrect quote concatenation. The intended table name
_SCHEMAis improperly escaped.🔎 Proposed fix
const tableExists = await conn.one( - "SELECT CASE WHEN EXISTS (SELECT 1 FROM user_tables WHERE table_name = '_SCHEMA') THEN 1 ELSE 0 END as cnt FROM DUAL", + "SELECT CASE WHEN EXISTS (SELECT 1 FROM user_tables WHERE table_name = :1) THEN 1 ELSE 0 END as cnt FROM DUAL", - [], + ['_SCHEMA'], a => a.cnt > 0 ).catch(() => false);Note: Oracle stores table names in uppercase by default, so you may need
UPPER('_SCHEMA')or check against uppercase.
1656-1656: Throw an Error object instead of a string.Line 1656 throws a plain string. Use
Parse.Errorfor consistency with the rest of the codebase.🔎 Proposed fix
default: - throw `Type ${schema.fields[fieldName].type} not supported yet`; + throw new Parse.Error( + Parse.Error.OPERATION_FORBIDDEN, + `Type ${schema.fields[fieldName].type} not supported yet` + );
🧹 Nitpick comments (2)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (2)
1-2697: Suggest Angular-style PR title for changelog.Based on learnings, Parse Server PRs should follow Angular commit conventions for meaningful changelog entries.
Suggested title:
feat(storage): add Oracle database adapter with SQL utilitiesThis clearly indicates it's a new feature in the storage subsystem, making it useful for developers reviewing the changelog.
Based on learnings.
1-2697: Documentation needed for Oracle adapter.Based on learnings, when adding new Parse Server features, documentation in README.md should be considered. This major feature (Oracle database support) should be documented with:
- Connection URI format (
oracle://...)- Configuration options specific to Oracle
- Any limitations or differences from MongoDB/PostgreSQL adapters
The PR objectives note that documentation updates are still needed.
Based on learnings.
Do you want me to draft documentation sections for the Oracle adapter?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.jsonsrc/Adapters/Storage/Oracle/OracleStorageAdapter.jssrc/Adapters/Storage/Oracle/sql/array/contains.sqlsrc/Controllers/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Adapters/Storage/Oracle/sql/array/contains.sql
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
📚 Learning: 2025-12-02T08:00:20.138Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
Applied to files:
package.json
📚 Learning: 2025-12-02T08:00:08.440Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:08.440Z
Learning: For Parse Server 9 release preparation, the parse/push-adapter dependency should be upgraded to version >= 8.0.0, not 7.x, as version 8.x is required despite dropping Node 18 support (which aligns with Parse Server 9's removal of EOL Node versions).
Applied to files:
package.json
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.
Applied to files:
src/Adapters/Storage/Oracle/OracleStorageAdapter.js
🧬 Code graph analysis (1)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (1)
src/Adapters/Storage/Oracle/OracleClient.js (1)
oracledb(2-2)
🪛 Biome (2.1.2)
src/Adapters/Storage/Oracle/OracleStorageAdapter.js
[error] 11-11: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 27-27: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 291-296: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 308-308: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 997-997: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1012-1012: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1014-1014: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1025-1025: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1053-1053: classes can only implement interfaces in TypeScript files
(parse)
[error] 1054-1054: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1055-1055: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1058-1058: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1059-1059: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1060-1060: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1061-1061: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1062-1062: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1063-1063: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1064-1064: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1064-1064: expected a semicolon to end the class property, but found none
(parse)
[error] 1064-1064: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '?'.
Expected an identifier, a string literal, a number literal, a private field name, or a computed name here.
(parse)
[error] 1065-1065: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 1067-1067: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1086-1086: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1086-1086: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1091-1091: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1091-1091: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1128-1128: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1149-1149: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1157-1157: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1157-1157: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1169-1169: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1170-1170: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1171-1171: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1172-1172: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1173-1173: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1173-1173: expected , but instead found ?
Remove ?
(parse)
[error] 1174-1174: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1244-1244: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1244-1244: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1244-1244: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1244-1244: expected , but instead found ?
Remove ?
(parse)
[error] 1267-1267: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1267-1267: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1267-1267: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1347-1347: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1347-1347: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1347-1347: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1366-1366: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1366-1366: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1366-1366: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1431-1431: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1431-1431: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1431-1431: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1440-1440: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1468-1468: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1468-1468: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1503-1503: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1503-1503: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1503-1503: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1503-1503: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1505-1505: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1505-1505: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1536-1536: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1550-1550: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1551-1551: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1552-1552: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1553-1553: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1553-1553: expected , but instead found ?
Remove ?
(parse)
[error] 1710-1710: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1711-1711: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1712-1712: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1713-1713: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1713-1713: expected , but instead found ?
Remove ?
(parse)
[error] 1757-1757: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1758-1758: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1759-1759: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1760-1760: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1761-1761: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1761-1761: expected , but instead found ?
Remove ?
(parse)
[error] 1762-1762: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1770-1770: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1771-1771: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1772-1772: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1773-1773: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1774-1774: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1774-1774: expected , but instead found ?
Remove ?
(parse)
[error] 1775-1775: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1944-1944: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1945-1945: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1946-1946: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1947-1947: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1948-1948: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1948-1948: expected , but instead found ?
Remove ?
(parse)
[error] 1961-1961: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1962-1962: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1963-1963: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1964-1964: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1989-1989: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2001-2001: expected ) but instead found :
Remove :
(parse)
[error] 2001-2001: Expected a statement but instead found '.'.
Expected a statement here.
(parse)
[error] 2049-2049: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2049-2049: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2049-2049: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2149-2149: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2149-2149: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2149-2149: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2180-2180: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2181-2181: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2182-2182: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2183-2183: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2183-2183: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2184-2184: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2184-2184: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2222-2222: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2222-2222: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2222-2222: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2222-2222: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2281-2281: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2282-2282: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2283-2283: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2284-2284: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2284-2284: expected , but instead found ?
Remove ?
(parse)
[error] 2285-2285: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2285-2285: expected , but instead found ?
Remove ?
(parse)
[error] 2285-2285: expected , but instead found mixed
Remove mixed
(parse)
[error] 2286-2286: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2286-2286: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2290-2290: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2291-2291: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2495-2495: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2534-2534: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2534-2534: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2534-2534: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2534-2534: expected , but instead found ?
Remove ?
(parse)
[error] 2534-2534: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2559-2559: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2560-2560: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2561-2561: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2562-2562: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2562-2562: expected , but instead found ?
Remove ?
(parse)
[error] 2563-2563: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2579-2579: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2579-2579: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2579-2579: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2579-2579: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2589-2589: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2594-2594: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2598-2598: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2602-2602: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2617-2617: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2617-2617: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2622-2622: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2622-2622: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2630-2630: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2631-2631: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2632-2632: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2633-2633: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2633-2633: expected , but instead found ?
Remove ?
(parse)
[error] 2634-2634: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2635-2635: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2635-2635: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2636-2636: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2639-2639: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2674-2674: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2674-2674: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2674-2674: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2685-2685: optional parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2685-2685: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 2685-2685: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 371-371: This number literal will lose precision at runtime.
The value at runtime will be 9223372036854776000
(lint/correctness/noPrecisionLoss)
[error] 699-699: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1503-1503: Shouldn't redeclare 'schema'. Consider to delete it or rename it.
'schema' is defined here:
(lint/suspicious/noRedeclare)
[error] 1758-1758: Shouldn't redeclare 'schema'. Consider to delete it or rename it.
'schema' is defined here:
(lint/suspicious/noRedeclare)
[error] 1759-1759: Shouldn't redeclare 'query'. Consider to delete it or rename it.
'query' is defined here:
(lint/suspicious/noRedeclare)
[error] 1771-1771: Shouldn't redeclare 'schema'. Consider to delete it or rename it.
'schema' is defined here:
(lint/suspicious/noRedeclare)
[error] 1772-1772: Shouldn't redeclare 'query'. Consider to delete it or rename it.
'query' is defined here:
(lint/suspicious/noRedeclare)
[error] 1779-1779: Shouldn't redeclare 'index'. Consider to delete it or rename it.
'index' is defined here:
(lint/suspicious/noRedeclare)
[error] 2631-2631: Shouldn't redeclare 'schema'. Consider to delete it or rename it.
'schema' is defined here:
(lint/suspicious/noRedeclare)
[error] 2634-2634: Shouldn't redeclare 'caseInsensitive'. Consider to delete it or rename it.
'caseInsensitive' is defined here:
(lint/suspicious/noRedeclare)
[error] 2637-2637: Shouldn't redeclare 'conn'. Consider to delete it or rename it.
'conn' is defined here:
(lint/suspicious/noRedeclare)
[error] 2675-2675: Shouldn't redeclare 'conn'. Consider to delete it or rename it.
'conn' is defined here:
(lint/suspicious/noRedeclare)
[error] 2686-2686: Shouldn't redeclare 'conn'. Consider to delete it or rename it.
'conn' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (4)
src/Controllers/index.js (1)
25-25: LGTM! Clean Oracle adapter integration.The Oracle storage adapter is properly imported and integrated following the same pattern as PostgreSQL. The URI-based routing in
getDatabaseAdapteris clean and consistent.Also applies to: 232-237
src/Adapters/Storage/Oracle/OracleStorageAdapter.js (2)
2495-2532: Well-structured initialization with Oracle SQL functions.The
performInitializationmethod properly sets up volatile class schemas and installs custom Oracle SQL functions (array operations, JSON helpers) from thesqlmodule. This is a clean approach to extending Oracle with Parse-specific functionality.
1-2697: Comprehensive test coverage required before merge.The PR objectives note that tests still need to be added. Given the complexity of this adapter (2697 lines), comprehensive test coverage is critical before merging. Tests should cover:
- Basic CRUD operations
- Schema management (create/update/delete classes and fields)
- Query operations (where clauses, comparators, special operators)
- Oracle-specific features (spatial queries, JSON operations)
- Transactions
- Index management
- Error handling and edge cases
- Connection pooling and lifecycle
Consider following the test patterns from PostgresStorageAdapter tests.
⛔ Skipped due to learnings
Learnt from: EmpiDev Repo: parse-community/parse-server PR: 9770 File: spec/CloudCode.spec.js:446-469 Timestamp: 2025-08-26T14:06:31.853Z Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.package.json (1)
53-53: No issues found.oracledb@^6.0.0is valid, fully compatible with the supported Node.js versions (all >=20.19.0 exceed the required >=14.6), and no known security vulnerabilities exist for this package.
| // @flow | ||
| import { createClient } from './OracleClient'; | ||
| // @flow-disable-next | ||
| import Parse from 'parse/node'; | ||
| // @flow-disable-next | ||
| import _ from 'lodash'; | ||
| // @flow-disable-next | ||
| import { v4 as uuidv4 } from 'uuid'; | ||
| import sql from './sql'; | ||
| import { StorageAdapter } from '../StorageAdapter'; | ||
| import type { SchemaType, QueryType, QueryOptions } from '../StorageAdapter'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
File uses Flow annotations in a .js file.
The file has Flow type annotations (@flow, import type, etc.) but uses a .js extension. This causes hundreds of static analysis errors. Parse Server appears to be a TypeScript project (has tsconfig.json, uses .ts files).
Either:
- Rename this file to
.tsand convert Flow annotations to TypeScript - Or remove all Flow annotations to make it a standard JavaScript file
Based on the project structure, converting to TypeScript is recommended.
🧰 Tools
🪛 Biome (2.1.2)
[error] 11-11: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js lines 1 to 11 the file
contains Flow annotations (// @flow, import type, etc.) but has a .js extension;
convert this file to TypeScript by renaming it to OracleStorageAdapter.ts and
replace Flow-specific syntax with TypeScript equivalents: remove // @flow
comments, change "import type { ... }" to TypeScript type imports or interfaces,
add proper type annotations on exports and function signatures, update any
default/commonjs imports to ES module types if needed, and ensure tsconfig
includes the path; run the TypeScript compiler and fix any type errors
introduced by the rename.
| const tableExists = await conn.one( | ||
| "SELECT CASE WHEN EXISTS (SELECT 1 FROM user_tables WHERE table_name = :1) THEN 1 ELSE 0 END as cnt FROM DUAL", | ||
| [className.toUpperCase()], | ||
| a => a.cnt > 0 | ||
| ).catch(() => false); |
There was a problem hiding this comment.
Systematic issue: Mixed bind parameter syntaxes.
The file mixes two incompatible bind parameter syntaxes:
- Oracle native (
:1,:2) - used in lines 1132, 1306, 1329, 1354, 1374, 1401, 1485, 1541, 2154, 2203, 2306, 2329, 2541, 2565, 2646 - pg-promise style (
$1,$2,$1:name) - used throughout buildWhereClause and CRUD operations
The OracleClient.js imports QueryFormatter which is designed for pg-promise syntax, but many raw queries use Oracle native bind syntax. These won't work together without conversion.
Resolution needed:
- Either consistently use pg-promise style and rely on QueryFormatter to convert
- Or use Oracle native binds throughout and remove QueryFormatter dependency
- The current mix will cause runtime failures
Verify how the client and formatter handle these different syntaxes.
Also applies to: 1329-1330, 1354-1356, 1374-1376, 1400-1403, 1485-1486, 1541-1543, 2154-2156, 2203-2203, 2306-2309, 2329-2331, 2541-2543, 2565-2567, 2645-2648
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js around lines 1305-1309
(and also at 1329-1330, 1354-1356, 1374-1376, 1400-1403, 1485-1486, 1541-1543,
2154-2156, 2203, 2306-2309, 2329-2331, 2541-2543, 2565-2567, 2645-2648) the code
mixes Oracle-native binds (e.g. :1) and pg-promise-style binds ($1/$1:name)
which are incompatible; pick one approach and make all queries consistent:
either (A) convert the raw SQL here to use pg-promise bind syntax ($1, $2,
$1:name) and rely on QueryFormatter to format/convert before execution, ensuring
all call sites and parameter arrays match that syntax, or (B) switch to
Oracle-native binds throughout and remove/disable QueryFormatter usage so
queries use :1/:2 consistently and parameter binding uses that client API;
update the listed queries to match the chosen syntax and run a quick pass to
verify the client/QueryFormatter integration (or removal) so no mixed bind
styles remain.
| if (where.pattern.length > 0 || !estimate) { | ||
| qs = `SELECT count(*) as cnt FROM $1:name ${wherePattern}`; | ||
| } else { | ||
| // Oracle approximate count | ||
| qs = "SELECT num_rows as approximate_row_count FROM user_tables WHERE table_name = :1"; | ||
| } | ||
|
|
||
| return this._client | ||
| .one(qs, values, a => { | ||
| if (a.approximate_row_count == null || a.approximate_row_count == -1) { | ||
| return !isNaN(+a.cnt) ? +a.cnt : 0; | ||
| } else { | ||
| return +a.approximate_row_count; | ||
| } | ||
| }) |
There was a problem hiding this comment.
Inconsistent bind parameter syntax for approximate count.
Lines 2203-2207 use Oracle's native :1 bind syntax, but the rest of the file uses pg-promise style ($1:name, $1, etc.). This query likely won't work with the QueryFormatter that's designed for pg-promise style parameters.
🔎 Proposed fix
if (where.pattern.length > 0 || !estimate) {
qs = `SELECT count(*) as cnt FROM $1:name ${wherePattern}`;
} else {
// Oracle approximate count
- qs = "SELECT num_rows as approximate_row_count FROM user_tables WHERE table_name = :1";
+ qs = "SELECT num_rows as approximate_row_count FROM user_tables WHERE table_name = $1";
}Ensure the parameter format matches the QueryFormatter expectations.
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleStorageAdapter.js around lines 2199-2213,
the approximate-count query uses Oracle native bind syntax ":1" which is
inconsistent with the file's pg-promise style parameters; change the bind to the
pg-promise format (use $1) so the QueryFormatter handles it, and ensure the same
values array (with the table name) is passed in as in the other branch; keep the
existing result-mapping logic unchanged.
Added comprehensive JSDoc comments to OracleClient, OracleConfigParser, OracleStorageAdapter, and QueryFormatter modules. These comments improve code readability and provide detailed documentation for functions, parameters, return values, and usage examples, aiding future maintenance and developer onboarding.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
src/Adapters/Storage/Oracle/QueryFormatter.js (1)
47-52: Security concern with:nameand:rawmodifiers remains valid.The
:namemodifier directly interpolatesparamValueas a quoted identifier, and:rawinjects raw SQL without any protection. This mirrors pg-promise's design where these modifiers are inherently unsafe if user-controlled input reaches these paths.This is a known pattern in pg-promise that requires careful caller discipline, but consider adding a brief comment or assertion to warn future maintainers.
src/Adapters/Storage/Oracle/OracleClient.js (4)
34-34: Inconsistent module format: ES6 export mixed with CommonJS require.Lines 1-3 use CommonJS
require()while line 34 uses ES6export function. Parse Server uses CommonJS throughout. Additionally, there's nomodule.exportsat the end of the file, so this export won't work correctly in a CommonJS environment.🔎 Proposed fix
-export function createClient(uri, databaseOptions) { +function createClient(uri, databaseOptions) { // ... existing code ... } + +module.exports = { createClient };
118-120: Nested tx() doesn't actually start a transaction.The
txmethod insidetaskContextjust calls the callback with the sametaskContext, which means no transaction semantics (commit/rollback) are applied. This could lead to data consistency issues if callers expect nested transaction behavior.🔎 Recommended options
tx: async (txName, callback) => { - return await callback(taskContext); + // Option 1: Delegate to outer tx for real transaction + return await client.tx(txName, callback); + // OR Option 2: Explicitly disallow + // throw new Error('Nested transactions are not supported within task()'); },
220-222: Silent error swallowing during rollback masks failures.Line 221 catches and ignores rollback errors with
.catch(() => {}). If the rollback fails (e.g., connection lost), this is hidden, making debugging difficult and potentially leaving the database in an inconsistent state.🔎 Proposed fix
} catch (error) { - await conn.rollback().catch(() => {}); + try { + await conn.rollback(); + } catch (rollbackError) { + // Log but don't throw to preserve original error + console.error('Oracle rollback failed:', rollbackError.message); + } throw error;
85-129: Significant code duplication between task(), tx(), and standalone methods.The
taskContext(lines 88-124),txContext(lines 182-216), and standalone methods (lines 130-178) contain nearly identical implementations ofany,one,none,map, andbatch. This violates DRY principles.Consider extracting a shared
createQueryContext(conn)factory function that returns the common query methods, then reuse it across task, tx, and standalone contexts.Also applies to: 179-226
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Adapters/Storage/Oracle/OracleClient.jssrc/Adapters/Storage/Oracle/OracleConfigParser.jssrc/Adapters/Storage/Oracle/OracleStorageAdapter.jssrc/Adapters/Storage/Oracle/QueryFormatter.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Adapters/Storage/Oracle/OracleConfigParser.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR #9938 and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.
📚 Learning: 2025-10-16T19:27:05.311Z
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.
Applied to files:
src/Adapters/Storage/Oracle/OracleClient.js
🔇 Additional comments (1)
src/Adapters/Storage/Oracle/QueryFormatter.js (1)
40-66: Parameter ordering implementation is correct.After careful analysis: the reverse-position iteration combined with sequential
paramIndexassignment and paralleloracleParams.push()produces correct bindings. When Oracle executes:1→params[0],:2→params[1], the values align correctly with the original$Nplaceholders.Example trace:
a = $1 AND b = $2with[100, 200]:
- $2 processed first →
:1, push 200- $1 processed second →
:2, push 100- Result:
a = :2 AND b = :1with[200, 100]- Oracle:
:1→200,:2→100 →a=100, b=200✓The previous review comment flagging this as a bug was incorrect.
| // Set Oracle client options | ||
| if (dbOptions.oracleClientOptions) { | ||
| for (const key in dbOptions.oracleClientOptions) { | ||
| oracledb[key] = dbOptions.oracleClientOptions[key]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Setting oracledb module properties globally affects all clients.
Directly modifying oracledb[key] sets properties on the shared module instance, which means these settings will affect all Oracle clients in the same process, not just this client instance. This could cause unexpected behavior if multiple clients with different oracleClientOptions are created.
Consider documenting this behavior or warning callers that these options are applied globally:
// Set Oracle client options
+ // Note: These options are applied globally to the oracledb module
+ // and will affect all Oracle connections in this process
if (dbOptions.oracleClientOptions) {🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around lines 46-51, the code
currently mutates the shared oracledb module by assigning oracledb[key] =
dbOptions.oracleClientOptions[key], which applies those settings process-wide;
instead, stop directly modifying the oracledb module: either (A) restrict
accepted options to ones that can be passed on a per-connection basis and apply
them when opening a connection, or (B) if you must set module-level properties,
capture the previous values, set them only while creating the client/connection
and then immediately restore the originals, and add a clear comment/warning in
the function signature and docs that options are global; implement one of these
approaches and remove the unconditional loop that writes into the oracledb
module.
| async connect(options) { | ||
| if (!pool) { | ||
| pool = await oracledb.createPool(poolConfig); | ||
| } | ||
| if (options && options.direct) { | ||
| // Direct connection (for schema notifications) | ||
| return await oracledb.getConnection(poolConfig); | ||
| } | ||
| return await pool.getConnection(); | ||
| }, |
There was a problem hiding this comment.
Race condition: concurrent calls to connect() can create multiple pools.
If connect() is called concurrently before the pool is initialized, multiple createPool() calls may execute simultaneously, leading to multiple pools being created and resource leaks.
🔎 Proposed fix using a promise-based lock
let pool = null;
+ let poolPromise = null;
const client = {
async connect(options) {
- if (!pool) {
- pool = await oracledb.createPool(poolConfig);
- }
+ if (!pool && !poolPromise) {
+ poolPromise = oracledb.createPool(poolConfig);
+ try {
+ pool = await poolPromise;
+ } finally {
+ poolPromise = null;
+ }
+ } else if (poolPromise) {
+ await poolPromise;
+ }
if (options && options.direct) {🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around lines 75 to 84, concurrent
calls to connect() can race and call oracledb.createPool() multiple times;
implement a promise-based lock: add a module-scoped initializingPromise (or
similar) that is set when createPool is first started, have subsequent callers
await that promise instead of calling createPool again, and when the createPool
promise resolves assign the singleton pool (and clear initializingPromise),
while on rejection clear initializingPromise and rethrow; also ensure
direct-option branch uses the resolved pool or awaits initializingPromise before
returning connections so only one pool is ever created.
| async none(query, params) { | ||
| const conn = await this.connect(); | ||
| try { | ||
| const formatted = formatQueryForOracle(query, params); | ||
| await conn.execute(formatted.query, formatted.params, { | ||
| outFormat: oracledb.OUT_FORMAT_OBJECT, | ||
| }); | ||
| } finally { | ||
| await conn.close(); | ||
| } |
There was a problem hiding this comment.
Missing autoCommit for none() operations outside transactions.
In standalone none() (lines 156-165), DML statements (INSERT/UPDATE/DELETE) will not be committed automatically since Oracle requires explicit commit. The connection is closed in the finally block without committing. Inside tx() context, commit happens at the end, but for standalone calls, changes may be lost.
🔎 Proposed fix for standalone none()
async none(query, params) {
const conn = await this.connect();
try {
const formatted = formatQueryForOracle(query, params);
await conn.execute(formatted.query, formatted.params, {
outFormat: oracledb.OUT_FORMAT_OBJECT,
+ autoCommit: true,
});
} finally {
await conn.close();
}
},Similarly, consider autoCommit: true for other standalone methods that may execute DML statements.
Also applies to: 199-204
🤖 Prompt for AI Agents
In src/Adapters/Storage/Oracle/OracleClient.js around lines 156-165 (and
similarly 199-204), standalone none() executes DML without committing; the
connection is simply closed so changes are not persisted. Modify the
conn.execute calls for standalone operations to pass { outFormat:
oracledb.OUT_FORMAT_OBJECT, autoCommit: true } (or enable autoCommit when not in
a transaction), and ensure tx() code path still controls commit/rollback so you
only set autoCommit for non-transactional executions.
| function formatQueryWithNamedParams(query, params) { | ||
| if (!params || params.length === 0) { | ||
| return { query, params: {} }; | ||
| } |
There was a problem hiding this comment.
Type inconsistency: function expects Object but checks Array length.
The JSDoc on line 74 states @param {Object} params - Object with named parameter values, but line 78 checks params.length === 0, which is an array check. For objects, this condition would not work as expected since objects don't have a length property (it would be undefined, making the check falsy).
Additionally, line 103 uses params[m.index - 1] which is array indexing, not object property access. The implementation appears to expect an array despite the documentation claiming it takes an object.
🔎 Proposed fix: Align documentation with implementation or fix the implementation
If the function should accept an array (like formatQuery):
/**
* Converts pg-promise style queries to Oracle named parameter format.
* Uses named bind variables (:name) instead of positional (:1, :2).
*
* @param {string} query - SQL query with pg-promise parameters
- * @param {Object} params - Object with named parameter values
+ * @param {Array} params - Array of parameter values
* @returns {Object} Object with formatted query and Oracle named parameters
*/
function formatQueryWithNamedParams(query, params) {
- if (!params || params.length === 0) {
+ if (!params || !Array.isArray(params) || params.length === 0) {
return { query, params: {} };
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
src/Adapters/Storage/Oracle/QueryFormatter.js around lines 77-80: fix the type
inconsistency by aligning the documentation with the implementation (the
function treats params as an array). Update the JSDoc @param to indicate params
is an Array of values (and adjust the description), and change the runtime check
to explicitly handle arrays (e.g., use Array.isArray(params) and params.length
=== 0). Ensure other code that indexes params (like the usage at line 103)
remains valid for an array; alternatively if you prefer object semantics,
convert the indexing logic to use named property access and replace the length
check with Object.keys(params).length === 0 — pick one approach and make the doc
and runtime code consistent.
bilalmohib
changed the title
bilalmohib
changed the title
I have fixed that now please check again |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5079 🏁 Script executed: Length of output: 127 @bilalmohib Great work on the fixes! I've verified the changes: ✅ Issue Link Fixed: The PR now correctly references issue #10000 ("Add Oracle 23ai storage adapter"), which exists and is open. This resolves the description check warning. ✅ JSDoc Comments Added: I can see from your commit that comprehensive JSDoc comments were added to the Oracle adapter modules (OracleClient, OracleConfigParser, OracleStorageAdapter, QueryFormatter). This should significantly improve docstring coverage. 📊 Docstring Coverage: While JSDoc comments were added, the automated docstring coverage check would need to run again to verify if the 43.75% has improved to meet the 80% threshold. The addition of comprehensive JSDoc to the four main Oracle modules should make a substantial impact. Remaining TasksBased on the PR description, you still have two items pending:
Recommendations
Would you like me to help with generating additional docstrings or reviewing any specific aspects of the implementation? 🧠 Learnings used |
Please check now I have fixed the code coverage. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 461 🏁 Script executed: Length of output: 5429 🏁 Script executed: Length of output: 3586 🏁 Script executed: Length of output: 252 🏁 Script executed: Length of output: 237 🏁 Script executed: Length of output: 156 🏁 Script executed: Length of output: 211 🏁 Script executed: Length of output: 50 🏁 Script executed: Length of output: 113 🏁 Script executed: Length of output: 1306 🏁 Script executed: Length of output: 1534 @bilalmohib Thank you for adding JSDoc comments! I can see that you've made good progress with your commit "Add JSDoc comments to Oracle adapter modules" (1fc412e). However, I notice that the walkthrough in your latest message still shows docstring coverage at 43.75%, which suggests that additional documentation may still be needed to reach the 80% threshold. Current Documentation StatusBased on my analysis of the Oracle adapter files:📊 Documentation found:
📈 What still needs documentation:
Next StepsWould you like me to help generate additional docstrings to improve coverage? You have two options:
I recommend option 1 for efficiency. Just let me know, and I'll generate the missing docstrings to help you reach the 80% coverage threshold! 🎯 🧠 Learnings used |
bilalmohib
changed the title
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
Added comprehensive JSDoc comments to all functions and methods in OracleStorageAdapter.js for improved documentation and maintainability. Introduced REMAINING_TASKS.md to track Oracle adapter PR tasks, security checks, error code mapping, and docstring coverage.
|
Closing, see #10000 (comment) |
3 participants
Introduces a new Oracle storage adapter, including a client implementation, configuration parser, and query formatter to support pg-promise style queries. Adds a set of Oracle SQL utility functions for array and JSON operations, and an index to load these SQL files. This enables Oracle as a backend option with feature parity for array and JSON manipulation.
Pull Request
Issue
Closes: #10000
Approach
This PR implements a fully functional Oracle 23ai storage adapter for Parse Server, modeled after the existing Postgres adapter. The implementation leverages Oracle 23ai's native JSON support and Oracle Spatial for comprehensive feature parity.
Architecture
OracleClient.js: Creates and manages Oracle connection pools using the
oracledbpackage. Provides a pg-promise-like interface for compatibility with existing Parse Server code, including methods for transactions, queries, and connection management.OracleConfigParser.js: Parses Oracle connection URIs and extracts configuration options including:
QueryFormatter.js: Translates pg-promise style queries (
$1:name,$2:raw) to Oracle bind variable syntax (:1,:"COLUMN_NAME"), maintaining compatibility with existing query building logic.OracleStorageAdapter.js: Main adapter class implementing the StorageAdapter interface with:
_SCHEMAtable synchronizationSQL Functions: Oracle-specific SQL functions for array and JSON operations:
array_add,array_add_unique,array_removearray_contains,array_contains_all,array_contains_all_regexjson_object_set_keyKey Design Decisions
EXISTSqueries instead ofCOUNT(*)for existence checks to improve performanceData Type Mappings
Testing Considerations
The adapter is designed to pass all standard Parse Server tests. Known limitations:
Tasks
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.