Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(oracle): add oracle dialect support #14638

Merged
merged 20 commits into from Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions dev/oracle/21-slim/wait-until-healthy.sh
@@ -1,5 +1,3 @@
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved

#!/usr/bin/env bash

if [ "$#" -ne 1 ]; then
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/abstract/query-generator.js
Expand Up @@ -1355,7 +1355,7 @@ class QueryGenerator {

// For the Oracle dialect, the result of a select is a set, not a sequence, and so is the result of UNION.
// So the top level ORDER BY is required
if (this._dialect.name !== 'oracle') {
if (!this._dialect.supports.topLevelOrderByRequired) {
delete options.order;
}
where[Op.placeholder] = true;
Expand Down
16 changes: 12 additions & 4 deletions src/dialects/oracle/connection-manager.js
Expand Up @@ -25,15 +25,23 @@ export class OracleConnectionManager extends AbstractConnectionManager {
this.sequelize = sequelize;
this.sequelize.config.port = this.sequelize.config.port || 1521;
this.lib = this._loadDialectModule('oracledb');
this.extendLib();
this.refreshTypeParser(DataTypes);
}

/**
* Method for initializing the lib
*
*/
extendLib() {
this.lib.maxRows = 1000;
sdepold marked this conversation as resolved.
Show resolved Hide resolved
if (sequelize.config && 'dialectOptions' in sequelize.config) {
const dialectOptions = sequelize.config.dialectOptions;
if (this.sequelize.config && 'dialectOptions' in this.sequelize.config) {
const dialectOptions = this.sequelize.config.dialectOptions;
if (dialectOptions && 'maxRows' in dialectOptions) {
this.lib.maxRows = sequelize.config.dialectOptions.maxRows;
this.lib.maxRows = this.sequelize.config.dialectOptions.maxRows;
}
if (dialectOptions && 'fetchAsString' in dialectOptions) {
this.lib.fetchAsString = sequelize.config.dialectOptions.fetchAsString;
this.lib.fetchAsString = this.sequelize.config.dialectOptions.fetchAsString;
} else {
this.lib.fetchAsString = [this.lib.CLOB];
}
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/oracle/index.js
Expand Up @@ -43,14 +43,14 @@ OracleDialect.prototype.supports = _.merge(_.cloneDeep(AbstractDialect.prototype
returnValues: false,
returnIntoValues: true,
'ORDER NULLS': true,
ignoreDuplicates: ' IGNORE',
schemas: true,
updateOnDuplicate: false,
indexViaAlter: false,
NUMERIC: true,
JSON: true,
upserts: true,
bulkDefault: true,
topLevelOrderByRequired: true,
GEOMETRY: false
});

Expand Down
4 changes: 2 additions & 2 deletions src/model.js
Expand Up @@ -2644,8 +2644,8 @@ class Model {
options.returning = true;
}
}

if (options.ignoreDuplicates && ['mssql', 'db2', 'oracle'].includes(dialect)) {
if (options.ignoreDuplicates && this.sequelize.dialect.supports.inserts.ignoreDuplicates === '' &&
Copy link
Member

Choose a reason for hiding this comment

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

Holy mother of Jesus, I didn't expect that thing to be a string. Since empty strings are false, I wonder if we should go with

options.ignoreDuplicates && !this.sequelize.dialect.supports.inserts.ignoreDuplicates

Why do we consider onConflictDoNothing suddenly?

Copy link
Author

Choose a reason for hiding this comment

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

Without adding onConflictDoNothing, I see this test case for postgresql was failing.

test/integration/model/bulk-create.test.js
 1) should support the ignoreDuplicates option

The test case enables ignoreDuplicates option under a check if either of property; ignoreDuplicates or onConflictDoNothing is non-null. Hence validating against only supports.inserts.ignoreDuplicates throws an error.

Error: postgres does not support the ignoreDuplicates option.

I modified to use ! for string empty check.
on the CI using latest version, can we add the lower version 18.4 in a separate PR?

this.sequelize.dialect.supports.inserts.onConflictDoNothing === '') {
throw new Error(`${dialect} does not support the ignoreDuplicates option.`);
}
if (options.updateOnDuplicate && (dialect !== 'mysql' && dialect !== 'mariadb' && dialect !== 'sqlite' && dialect !== 'postgres')) {
Expand Down
7 changes: 6 additions & 1 deletion test/integration/associations/belongs-to-many.test.js
Expand Up @@ -1417,7 +1417,12 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), () => {

describe('hasAssociations with binary key', () => {
beforeEach(function() {
const keyDataType = ['mysql', 'mariadb', 'db2'].includes(dialect) ? 'BINARY(255)' : dialect === 'oracle' ? DataTypes.STRING(255, true) : DataTypes.BLOB('tiny');
let keyDataType = DataTypes.BLOB('tiny');
if (['mysql', 'mariadb', 'db2'].includes(dialect)) {
keyDataType = 'BINARY(255)';
} else if (dialect === 'oracle') {
keyDataType = DataTypes.STRING(255, true);
}
this.Article = this.sequelize.define('Article', {
id: {
type: keyDataType,
Expand Down