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 2 commits
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
27 changes: 27 additions & 0 deletions .github/workflows/ci.yml
Expand Up @@ -47,6 +47,33 @@ jobs:
- run: yarn install --frozen-lockfile
- run: yarn add --dev typescript@~${{ matrix.ts-version }}
- run: yarn test-typings
test-oracle:
strategy:
fail-fast: false
matrix:
node-version: [10, 16]
Copy link
Member

Choose a reason for hiding this comment

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

You can bump it to 18 now, we still need to update the v6 ci :)

Suggested change
node-version: [10, 16]
node-version: [10, 18]

Copy link
Member

Choose a reason for hiding this comment

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

@ephys should I update the CI to be similar to main where we have the separate steps?

Choose a reason for hiding this comment

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

We have updated the node version to 18 for the Oracle dialect

name: Oracle DB (Node ${{ matrix.node-version }})
runs-on: ubuntu-latest
env:
DIALECT: oracle
SEQ_ORACLE_USER: sequelizetest
SEQ_ORACLE_PW: sequelizepassword
SEQ_ORACLE_DB: XEPDB1
SEQ_ORACLE_HOST: localhost
SEQ_ORACLE_PORT: 1521
UV_THREADPOOL_SIZE: 128
steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
Copy link
Member

Choose a reason for hiding this comment

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

Also something we have neglected to update 😅

Suggested change
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
- uses: actions/checkout@v3
- uses: actions/setup-node@v3

Copy link
Author

Choose a reason for hiding this comment

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

Done

with:
node-version: ${{ matrix.node-version }}
- run: yarn install --frozen-lockfile --ignore-engines
- name: Install Local Oracle DB
run: yarn start-oracle
- name: Unit Tests
run: yarn test-unit
- name: Integration Tests
run: yarn test-integration-oracle
Copy link
Member

Choose a reason for hiding this comment

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

What is the current oldest version that is not EOL? Can you add a test against that one too?

Copy link
Author

Choose a reason for hiding this comment

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

With Sequelize, we are planning to support DB version 18.4 and above. currently, we have test (start.sh) for 21.3 DB version and folder is named as latest to fetch latest version always. But as you mentioned to use fixed versions, I shall rename this to 21c. You wanted to add test case for 18.4 too ?

Copy link
Author

Choose a reason for hiding this comment

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

dev/oracle/21-slim/ folder is added to indicate 21c DB version is used as a Oracle docker image.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
run: yarn test-integration-oracle
run: yarn test-integration

I think that if we specify LD_LIBRARY_PATH in the env variables above this should also work, just for consistency

Copy link
Author

Choose a reason for hiding this comment

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

Yes I have set the LD_LIBRARY_PATH and it is getting populated in test-integration

test-db2:
strategy:
fail-fast: false
Expand Down
17 changes: 17 additions & 0 deletions dev/oracle/latest/docker-compose.yml
@@ -0,0 +1,17 @@
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved

services:
oraclexedb:
container_name: oraclexedb
image: gvenzl/oracle-xe:latest
Copy link
Member

Choose a reason for hiding this comment

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

We don't use :latest anymore (though the v6 branch is a bit outdated), we'll make renovate watch this file so it can open a PR when a new major is released :)

Suggested change
image: gvenzl/oracle-xe:latest
image: gvenzl/oracle-xe:21.3.0

Copy link
Member

Choose a reason for hiding this comment

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

We do have to tell renovate to also check the v6 branch though, but that's something we as maintainers will do.

Choose a reason for hiding this comment

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

Done

environment:
ORACLE_PASSWORD: password
ports:
- 1521:1521
healthcheck:
test: ["CMD-SHELL", "sqlplus", "system/password@XEPDB1"]
retries: 10

networks:
default:
name: sequelize-oraclexedb-network
27 changes: 27 additions & 0 deletions dev/oracle/latest/privileges.sql
@@ -0,0 +1,27 @@
-- Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved

create user sequelizetest identified by sequelizepassword;
grant connect to sequelizetest with admin option;
grant create session to sequelizetest with admin option;
grant grant any privilege to sequelizetest with admin option;
grant grant any role to sequelizetest with admin option;
grant create any table to sequelizetest with admin option;
grant insert any table to sequelizetest with admin option;
grant select any table to sequelizetest with admin option;
grant update any table to sequelizetest with admin option;
grant delete any table to sequelizetest with admin option;
grant drop any table to sequelizetest with admin option;
grant create view to sequelizetest with admin option;
grant create user to sequelizetest with admin option;
grant drop user to sequelizetest with admin option;
grant create any trigger to sequelizetest with admin option;
grant create any procedure to sequelizetest with admin option;
grant create any sequence to sequelizetest with admin option;
grant select any sequence to sequelizetest with admin option;
grant drop any sequence to sequelizetest with admin option;
grant create any synonym to sequelizetest with admin option;
grant create any index to sequelizetest with admin option;
grant alter user to sequelizetest with admin option;
grant alter any table to sequelizetest with admin option;
alter user sequelizetest quota unlimited on users;
exit;
44 changes: 44 additions & 0 deletions dev/oracle/latest/start.sh
@@ -0,0 +1,44 @@
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved

#!/usr/bin/env bash
set -Eeuxo pipefail # https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/
cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" # https://stackoverflow.com/a/17744637

# Remove an existing Oracle DB docker image
docker-compose -p oraclexedb down --remove-orphans

# Bring up new Oracle DB docker image
docker-compose -p oraclexedb up -d

# Wait until Oracle DB is set up and docker state is healthy
./wait-until-healthy.sh oraclexedb

# Moving privileges.sql to docker container
docker cp privileges.sql oraclexedb:/opt/oracle/.

# Granting all the needed privileges to sequelizetest user
docker exec -t oraclexedb sqlplus system/password@XEPDB1 @privileges.sql

# Setting up Oracle instant client for oracledb
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed to run our tests? :o

Is ~/oracle the recommended folder? I would prefer to not add any file outside of the sequelize directory. Can we move it to a gitignored folder called .oracle at the project root?

Copy link
Member

Choose a reason for hiding this comment

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

If this is needed, is there also a version for Windows that should be added here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes we need to install the Instant client for oracle dialect to work. The privileges are explicitly provided, so that the tests dont just run under higher privileges (like SYSDBA) but under the required privileges. I have moved the installation to Project root/ .oracle folder. I added the .oracle to the .gitignore list. Windows support is added .

if [ ! -d ~/oracle ] && [ $(uname) == 'Linux' ]
then
mkdir ~/oracle &&
wget https://download.oracle.com/otn_software/linux/instantclient/instantclient-basic-linuxx64.zip --no-check-certificate &&
unzip instantclient-basic-linuxx64.zip -d ~/oracle/ &&
rm instantclient-basic-linuxx64.zip &&
mv ~/oracle/instantclient_21_6 ~/oracle/instantclient

echo "Local Oracle instant client has been setup!"
elif [ ! -d ~/Downloads/instantclient_19_8 ] && [ $(uname) == 'Darwin' ]
then
curl -O https://download.oracle.com/otn_software/mac/instantclient/instantclient-basic-macos.dmg &&
hdiutil mount instantclient-basic-macos.dmg &&
/Volumes/instantclient-basic-macos.x64-19.8.0.0.0dbru/install_ic.sh &&
hdiutil unmount /Volumes/instantclient-basic-macos.x64-19.8.0.0.0dbru &&
rm instantclient-basic-macos.dmg &&
ln -s ~/Downloads/instantclient_19_8/libclntsh.dylib ../../../node_modules/oracledb/build/Release/

echo "Local Oracle instant client has been setup!"
fi

echo "Local Oracle DB is ready for use!"
10 changes: 10 additions & 0 deletions dev/oracle/latest/stop.sh
@@ -0,0 +1,10 @@
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved

#!/usr/bin/env bash
set -Eeuxo pipefail # https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/
cd -P -- "$(dirname -- "${BASH_SOURCE[0]}")" # https://stackoverflow.com/a/17744637


docker-compose -p oraclexedb down --remove-orphans

echo "Local Oracle DB instance stopped (if it was running)."
23 changes: 23 additions & 0 deletions dev/oracle/latest/wait-until-healthy.sh
@@ -0,0 +1,23 @@
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved

#!/usr/bin/env bash

if [ "$#" -ne 1 ]; then
>&2 echo "Please provide the container name or hash"
exit 1
fi

for _ in {1..50}
do
state=$(docker inspect -f '{{ .State.Health.Status }}' $1 2>&1)
return_code=$?
if [ ${return_code} -eq 0 ] && [ "$state" == "healthy" ]; then
echo "$1 is healthy!"
sleep 60
exit 0
fi
sleep 6
done

>&2 echo "Timeout of 5m exceeded when waiting for container to be healthy: $1"
exit 1
11 changes: 10 additions & 1 deletion package.json
Expand Up @@ -106,6 +106,7 @@
"mysql2": "^2.3.3",
"node-hook": "^1.0.0",
"nyc": "^15.1.0",
"oracledb": "^5.4.0",
WikiRik marked this conversation as resolved.
Show resolved Hide resolved
"p-map": "^4.0.0",
"p-props": "^4.0.0",
"p-settle": "^4.1.1",
Expand Down Expand Up @@ -160,6 +161,7 @@
"db2",
"ibm_db",
"sql",
"oracledb",
"sqlserver",
"snowflake",
"orm",
Expand Down Expand Up @@ -239,17 +241,20 @@
"start-postgres": "bash dev/postgres/10/start.sh",
"start-mssql": "bash dev/mssql/2019/start.sh",
"start-db2": "bash dev/db2/11.5/start.sh",
"start-oracle": "bash dev/oracle/latest/start.sh",
"stop-mariadb": "bash dev/mariadb/10.3/stop.sh",
"stop-mysql": "bash dev/mysql/5.7/stop.sh",
"stop-mysql-8": "bash dev/mysql/8.0/stop.sh",
"stop-postgres": "bash dev/postgres/10/stop.sh",
"stop-mssql": "bash dev/mssql/2019/stop.sh",
"stop-db2": "bash dev/db2/11.5/stop.sh",
"stop-oracle": "bash dev/oracle/latest/stop.sh",
"restart-mariadb": "npm run start-mariadb",
"restart-mysql": "npm run start-mysql",
"restart-postgres": "npm run start-postgres",
"restart-mssql": "npm run start-mssql",
"restart-db2": "npm run start-db2",
"restart-oracle": "npm run start-oracle",
"----------------------------------------- local tests ---------------------------------------------": "",
"test-unit-mariadb": "cross-env DIALECT=mariadb npm run test-unit",
"test-unit-mysql": "cross-env DIALECT=mysql npm run test-unit",
Expand All @@ -259,7 +264,8 @@
"test-unit-mssql": "cross-env DIALECT=mssql npm run test-unit",
"test-unit-db2": "cross-env DIALECT=db2 npm run test-unit",
"test-unit-snowflake": "cross-env DIALECT=snowflake npm run test-unit",
"test-unit-all": "npm run test-unit-mariadb && npm run test-unit-mysql && npm run test-unit-postgres && npm run test-unit-postgres-native && npm run test-unit-mssql && npm run test-unit-sqlite && npm run test-unit-snowflake && npm run test-unit-db2",
"test-unit-all": "npm run test-unit-mariadb && npm run test-unit-mysql && npm run test-unit-postgres && npm run test-unit-postgres-native && npm run test-unit-mssql && npm run test-unit-sqlite && npm run test-unit-snowflake && npm run test-unit-db2 && npm run test-unit-oracle",
"test-unit-oracle": "cross-env DIALECT=oracle npm run test-unit",
sdepold marked this conversation as resolved.
Show resolved Hide resolved
"test-integration-mariadb": "cross-env DIALECT=mariadb npm run test-integration",
"test-integration-mysql": "cross-env DIALECT=mysql npm run test-integration",
"test-integration-postgres": "cross-env DIALECT=postgres npm run test-integration",
Expand All @@ -268,13 +274,15 @@
"test-integration-mssql": "cross-env DIALECT=mssql npm run test-integration",
"test-integration-db2": "cross-env DIALECT=db2 npm run test-integration",
"test-integration-snowflake": "cross-env DIALECT=snowflake npm run test-integration",
"test-integration-oracle": "cross-env LD_LIBRARY_PATH=$HOME/oracle/instantclient/ DIALECT=oracle UV_THREADPOOL_SIZE=128 npm run test-integration",
Copy link
Member

Choose a reason for hiding this comment

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

I'm using GitHub Codespaces, running Node 16 and starting Oracle seems to work fine but I can't run integration tests.

 SequelizeConnectionError: DPI-1047: Cannot locate a 64-bit Oracle Client library: "libaio.so.1: cannot open shared object file: No such file or directory". See https://oracle.github.io/node-oracledb/INSTALL.html for help
Node-oracledb installation instructions: https://oracle.github.io/node-oracledb/INSTALL.html
You must have 64-bit Oracle Client libraries configured with ldconfig, or in LD_LIBRARY_PATH.
If you do not have Oracle Database on this computer, then install the Instant Client Basic or Basic Light package from 
https://www.oracle.com/database/technologies/instant-client/linux-x86-64-downloads.html

Copy link
Author

Choose a reason for hiding this comment

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

The libaio package needs to be added manually and is not part of the instant client.
It looks like its not installed in VM/container?
The instructions details are here :
https://oracle.github.io/node-oracledb/INSTALL.html (grep for libaio )

"test-mariadb": "cross-env DIALECT=mariadb npm test",
"test-mysql": "cross-env DIALECT=mysql npm test",
"test-sqlite": "cross-env DIALECT=sqlite npm test",
"test-postgres": "cross-env DIALECT=postgres npm test",
"test-postgres-native": "cross-env DIALECT=postgres-native npm test",
"test-mssql": "cross-env DIALECT=mssql npm test",
"test-db2": "cross-env DIALECT=db2 npm test",
"test-oracle": "cross-env LD_LIBRARY_PATH=$HOME/oracle/instantclient/ DIALECT=oracle UV_THREADPOOL_SIZE=128 npm test",
"----------------------------------------- development ---------------------------------------------": "",
"sscce": "node sscce.js",
"sscce-mariadb": "cross-env DIALECT=mariadb node sscce.js",
Expand All @@ -284,6 +292,7 @@
"sscce-sqlite": "cross-env DIALECT=sqlite node sscce.js",
"sscce-mssql": "cross-env DIALECT=mssql node sscce.js",
"sscce-db2": "cross-env DIALECT=db2 node sscce.js",
"sscce-oracle": "cross-env LD_LIBRARY_PATH=$HOME/oracle/instantclient/ DIALECT=oracle node sscce.js",
"prepare": "npm run build && husky install",
"build": "node ./build.js",
"---------------------------------------------------------------------------------------------------": ""
Expand Down
1 change: 1 addition & 0 deletions src/data-types.js
Expand Up @@ -1060,6 +1060,7 @@ dialectMap.sqlite = require('./dialects/sqlite/data-types')(DataTypes);
dialectMap.mssql = require('./dialects/mssql/data-types')(DataTypes);
dialectMap.db2 = require('./dialects/db2/data-types')(DataTypes);
dialectMap.snowflake = require('./dialects/snowflake/data-types')(DataTypes);
dialectMap.oracle = require('./dialects/oracle/data-types')(DataTypes);

const dialectList = Object.values(dialectMap);

Expand Down
57 changes: 49 additions & 8 deletions src/dialects/abstract/query-generator.js
Expand Up @@ -88,6 +88,16 @@ class QueryGenerator {
return `ALTER TABLE ${this.quoteTable(before)} RENAME TO ${this.quoteTable(after)};`;
}

/**
* Helper method for getting the returning into bind information
* that is needed by some dialects (currently Oracle)
*
* @private
*/
getInsertQueryReturnIntoBinds() {
// noop by default
}

/**
* Returns an insert into command
*
Expand All @@ -103,12 +113,14 @@ class QueryGenerator {
_.defaults(options, this.options);

const modelAttributeMap = {};
const bind = [];
const bind = options.bind || [];
sdepold marked this conversation as resolved.
Show resolved Hide resolved
const fields = [];
const returningModelAttributes = [];
const returnTypes = [];
const values = [];
const quotedTable = this.quoteTable(table);
const bindParam = options.bindParam === undefined ? this.bindParam(bind) : options.bindParam;
const returnAttributes = [];
let query;
let valueQuery = '';
let emptyQuery = '';
Expand All @@ -132,10 +144,14 @@ class QueryGenerator {
emptyQuery += ' VALUES ()';
}

if (this._dialect.supports.returnValues && options.returning) {
if ((this._dialect.supports.returnValues || this._dialect.supports.returnIntoValues) && options.returning) {
const returnValues = this.generateReturnValues(modelAttributes, options);

returningModelAttributes.push(...returnValues.returnFields);
// Storing the returnTypes for dialects that need to have returning into bind information for outbinds
if (this._dialect.supports.returnIntoValues) {
returnTypes.push(...returnValues.returnTypes);
}
returningFragment = returnValues.returningFragment;
tmpTable = returnValues.tmpTable || '';
outputFragment = returnValues.outputFragment || '';
Expand Down Expand Up @@ -244,7 +260,12 @@ class QueryGenerator {
emptyQuery += returningFragment;
}

query = `${replacements.attributes.length ? valueQuery : emptyQuery};`;
if (this._dialect.supports.returnIntoValues && options.returning) {
// Populating the returnAttributes array and performing operations needed for output binds of insertQuery
this.getInsertQueryReturnIntoBinds(returnAttributes, bind.length, returningModelAttributes, returnTypes, options);
Copy link
Member

Choose a reason for hiding this comment

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

Hm. What is this doing? As far as I can tell, we never override getInsertQueryReturnIntoBinds anywhere and the defined abstract function is a noop?

Copy link

Choose a reason for hiding this comment

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

For the Oracle dialect we generate sql statement like these - INSERT INTO "Campuses" ("id","name","address","description","createdAt","updatedAt") VALUES (DEFAULT,:1,:2,:3,:4,:5) RETURNING "id","name","address","description","createdAt","updatedAt" INTO :6,:7,:8,:9,:10,:11;

getInsertQueryReturnIntoBinds function generates the bind values for outBinds (:6,:7,:8,:9,:10,:11 in the above sql) , generates the bindDefs for the out binds and also populates the options.outBindAttributes field.

Yes, this function is only used by the Oracle dialect, for now, we have defined an abstract function with a noop so that other dialects can override it if required.

}

query = `${replacements.attributes.length ? valueQuery : emptyQuery}${returnAttributes.join(',')};`;
Copy link
Member

Choose a reason for hiding this comment

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

returnAttributes is empty for the other dialects right?

Choose a reason for hiding this comment

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

Yes, it would be empty for other dialects.

if (this._dialect.supports.finalTable) {
query = `SELECT * FROM FINAL TABLE(${ replacements.attributes.length ? valueQuery : emptyQuery });`;
}
Expand Down Expand Up @@ -383,8 +404,18 @@ class QueryGenerator {
const bindParam = options.bindParam === undefined ? this.bindParam(bind) : options.bindParam;

if (this._dialect.supports['LIMIT ON UPDATE'] && options.limit) {
if (this.dialect !== 'mssql' && this.dialect !== 'db2') {
if (this.dialect !== 'mssql' && this.dialect !== 'db2' && this.dialect !== 'oracle') {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (this.dialect !== 'mssql' && this.dialect !== 'db2' && this.dialect !== 'oracle') {
if (!['mssql','db2','oracle'].includes(this.dialect)) {

Copy link
Member

Choose a reason for hiding this comment

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

About this: I'm fine with it being done in tests but I'd rather avoid doing it in production code. I don't like creating throwaway objects in code that is executed often like here

Copy link
Author

Choose a reason for hiding this comment

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

Added suggested change

suffix = ` LIMIT ${this.escape(options.limit)} `;
} else if (this.dialect === 'oracle') {
//This cannot be setted in where because rownum will be quoted
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//This cannot be setted in where because rownum will be quoted
// This cannot be setted in where because rownum will be quoted

Not a high priority, but I prefer a space between the // and the start of the comment. I won't make this suggestion in other places

Copy link
Author

Choose a reason for hiding this comment

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

Done

if (where && (where.length && where.length > 0 || Object.keys(where).length > 0)) {
//If we have a where clause, we add AND
suffix += ' AND ';
} else {
//No where clause, we add where
suffix += ' WHERE ';
}
suffix += `rownum <= ${this.escape(options.limit)} `;
}
}

Expand Down Expand Up @@ -1188,6 +1219,7 @@ class QueryGenerator {
let mainJoinQueries = [];
let subJoinQueries = [];
let query;
const hasAs = this._dialect.name === 'oracle' ? '' : 'AS ';

// Aliases can be passed through subqueries and we don't want to reset them
if (this.options.minifyAliases && !options.aliasesMapping) {
Expand Down Expand Up @@ -1312,7 +1344,12 @@ class QueryGenerator {
} else {
// Ordering is handled by the subqueries, so ordering the UNION'ed result is not needed
groupedLimitOrder = options.order;
delete options.order;

// 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') {
sdepold marked this conversation as resolved.
Show resolved Hide resolved
delete options.order;
}
where[Op.placeholder] = true;
}

Expand All @@ -1332,7 +1369,7 @@ class QueryGenerator {
model
},
model
).replace(/;$/, '')}) AS sub`; // Every derived table must have its own alias
).replace(/;$/, '')}) ${hasAs}sub`; // Every derived table must have its own alias
const placeHolder = this.whereItemQuery(Op.placeholder, true, { model });
const splicePos = baseQuery.indexOf(placeHolder);

Expand Down Expand Up @@ -1426,7 +1463,7 @@ class QueryGenerator {

if (subQuery) {
this._throwOnEmptyAttributes(attributes.main, { modelName: model && model.name, as: mainTable.as });
query = `SELECT ${attributes.main.join(', ')} FROM (${subQueryItems.join('')}) AS ${mainTable.as}${mainJoinQueries.join('')}${mainQueryItems.join('')}`;
query = `SELECT ${attributes.main.join(', ')} FROM (${subQueryItems.join('')}) ${hasAs}${mainTable.as}${mainJoinQueries.join('')}${mainQueryItems.join('')}`;
} else {
query = mainQueryItems.join('');
}
Expand Down Expand Up @@ -1566,6 +1603,8 @@ class QueryGenerator {
prefix = `(${this.quoteIdentifier(includeAs.internalAs)}.${attr.replace(/\(|\)/g, '')})`;
} else if (/json_extract\(/.test(attr)) {
prefix = attr.replace(/json_extract\(/i, `json_extract(${this.quoteIdentifier(includeAs.internalAs)}.`);
} else if (/json_value\(/.test(attr)) {
prefix = attr.replace(/json_value\(/i, `json_value(${this.quoteIdentifier(includeAs.internalAs)}.`);
} else {
prefix = `${this.quoteIdentifier(includeAs.internalAs)}.${this.quoteIdentifier(attr)}`;
}
Expand Down Expand Up @@ -1822,6 +1861,8 @@ class QueryGenerator {

if (this._dialect.supports.returnValues.returning) {
returningFragment = ` RETURNING ${returnFields.join(',')}`;
} else if (this._dialect.supports.returnIntoValues) {
returningFragment = ` RETURNING ${returnFields.join(',')} INTO `;
} else if (this._dialect.supports.returnValues.output) {
outputFragment = ` OUTPUT ${returnFields.map(field => `INSERTED.${field}`).join(',')}`;

Expand All @@ -1835,7 +1876,7 @@ class QueryGenerator {
}
}

return { outputFragment, returnFields, returningFragment, tmpTable };
return { outputFragment, returnFields, returnTypes, returningFragment, tmpTable };
}

generateThroughJoin(include, includeAs, parentTableName, topLevelInfo) {
Expand Down