Support crud operations - #566
Conversation
|
👋 A new build is available for this PR based on ef8dcef. |
| const identityColumns = columns.filter(column => column.IS_IDENTITY === `YES`); | ||
| const insertColumns = columns.filter(column => column.IS_IDENTITY !== `YES`); |
There was a problem hiding this comment.
I'm not sure if just looking at whether or not the column is an identity column is sufficient.
For example, would this code handle the below example well? There are a few different generated columns that are not identity columns. Perhaps you also need to take into account whether the column has a default (HAS_DEFAULT column in SYSCOLUMNS).
-- If I create this table on COMMON76:
CREATE OR REPLACE TABLE RMOELLER.MYTABLE (
C1 INT GENERATED BY DEFAULT AS IDENTITY,
C2 TIMESTAMP GENERATED FOR EACH ROW ON UPDATE AS ROW CHANGE TIMESTAMP NOT NULL,
C3 TIMESTAMP(12) GENERATED AS ROW BEGIN NOT NULL,
C3E TIMESTAMP(12) GENERATED AS ROW END NOT NULL,
C4 TIMESTAMP(12) GENERATED AS TRANSACTION START ID NOT NULL,
PERIOD FOR SYSTEM_TIME (C3, C3E)
)
ON REPLACE DELETE ROWS;
-- IS_IDENTITY is only 'YES' for column C1
SELECT COLUMN_NAME,
IS_IDENTITY,
HAS_DEFAULT
FROM QSYS2.SYSCOLUMNS
WHERE TABLE_SCHEMA = 'RMOELLER'
AND TABLE_NAME = 'MYTABLE';
-- ACS generates the following:
INSERT INTO RMOELLER.MYTABLE (
C1, /* C1 INTEGER */
C2, /* C2 TIMESTAMP */
C3, /* C3 TIMESTAMP(12) */
C3E, /* C3E TIMESTAMP(12) */
C4 /* C4 TIMESTAMP(12) */
)
VALUES (
DEFAULT, /* INTEGER Generated Value: Identity */
DEFAULT, /* TIMESTAMP Generated Value: Row change */
DEFAULT, /* TIMESTAMP(12) Generated Value: Row begin */
DEFAULT, /* TIMESTAMP(12) Generated Value: Row end */
DEFAULT /* TIMESTAMP(12) Generated Value: Transaction start ID */
);
| const identityColumns = columns.filter(column => column.IS_IDENTITY === `YES`); | ||
| const insertColumns = columns.filter(column => column.IS_IDENTITY !== `YES`); | ||
|
|
||
| if (insertColumns.length === 0) { |
There was a problem hiding this comment.
You can still insert a row into the table if it contains exclusively identity columns, you just need to insert all DEFAULT values -- see my comment above. I'm not sure if we will ever encounter a situation where an insert statement cannot be generated. Do we need this error message?
| ` cst.CONSTRAINT_SCHEMA = key.CONSTRAINT_SCHEMA and`, | ||
| ` cst.CONSTRAINT_NAME = key.CONSTRAINT_NAME`, | ||
| `WHERE cst.CONSTRAINT_TYPE in ('PRIMARY KEY', 'UNIQUE')`, | ||
| ` AND cst.TABLE_SCHEMA = ?`, |
There was a problem hiding this comment.
You check for the long and short table names - should you also be checking for the long and short schema/library name here?
…em schema names; organized test cases
|
@ryan-moeller21 I've integrated the default type as implemented in ACS; as you can see, this is the output I get based on your table: 
I've added a check on the schema name to support both system names and long names: |
| ...columnList(columns), | ||
| `)`, | ||
| `VALUES (`, | ||
| ...columns.map((column, index) => ` DEFAULT${index < columns.length - 1 ? `,` : ``} -- ${describe(column)} - ${generatedKind(column)}`), |
There was a problem hiding this comment.
Instead of just generating comments, I would say that we should like ACS make use of host variables here:
There was a problem hiding this comment.
I wonder if in addition to the query with host variables, we then include a bind statement below it with some placeholder data based on the columns. So for the user it would be as easy as just edit the values and run it.
Just an idea. I would also be fine with just having the host variables with no bind statement.
| ...describeAll(setColumns), | ||
| `WHERE`, | ||
| ...where.lines, |
There was a problem hiding this comment.
For line 65, I would say we shouldn't comment out the column names.
For line 67, again what do you think about adding host variables. And bind statement?
|
|
||
| function whereClause(schema: string, name: string, columns: TableColumn[], keyColumns: string[]) { | ||
| // Without a key, every column is listed so the statement never matches more rows than intended | ||
| const predicateColumns = keyColumns.length ? columns.filter(column => isKeyColumn(column, keyColumns)) : columns; |
There was a problem hiding this comment.
ACS currently prompts in the case it has a key:
Rather than prompting, what do you think about adding an extension setting? Basically to configure whether to use a key constraint (if found and if not found fallback to all columns) or always use all columns.
| ...where.warnings, | ||
| `DELETE FROM ${qualify(schema, name)}`, | ||
| `WHERE`, | ||
| ...where.lines, |
There was a problem hiding this comment.
Again thoughts on host variables here and bind statement following?
|
Good morning @SanjulaGanepola, 
I've also implemented a configuration for keys, in this way we can simulate like the file has no keys: |
|
👋 A new build is available for this PR based on eca2122. |
3 participants
Changes
This PR allows you to generate INSERT/DELETE/UPDATE statements for tables.
How to test this PR
From the SCHEMA BROWSER, select a a table, now you'll see "INSERT/DELETE/UPDATE" under Generate SQL menu:
Insert example:
Update example, no primary key detected:
Delete example:
@forstie @ryan-moeller21 could you test it?
Checklist
console.logs I addedCloses #141