Skip to content

Commit

Permalink
Fix an issue in table dialog where changing column name was not synci…
Browse files Browse the repository at this point in the history
…ng table constraints appropriately. #7229
  • Loading branch information
adityatoshniwal committed Mar 8, 2024
1 parent 134e651 commit 7374997
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,12 @@ class ExclusionColHeaderSchema extends BaseUISchema {

/* Data to ExclusionColumnSchema will added using the header form */
getNewData(data) {
let colType = data.is_exp ? null : _.find(this.columnOptions, (col)=>col.value==data.column)?.datatype;
const column = _.find(this.columnOptions, (col)=>col.value==data.column);
return this.exColumnSchema.getNewData({
is_exp: data.is_exp,
column: data.is_exp ? data.expression : data.column,
col_type: colType,
column_cid: data.is_exp ? null : column?.cid,
col_type: data.is_exp ? null : column?.datatype,
});
}

Expand Down Expand Up @@ -370,20 +371,21 @@ export default class ExclusionConstraintSchema extends BaseUISchema {
depChange: (state, source, topState, actionObj)=>{
/* If in table, sync up value with columns in table */
if(obj.inTable && !state) {
/* the FK is removed by some other dep, this can be a no-op */
/* the constraint is removed by some other dep, this can be a no-op */
return;
}

let currColumns = state.columns || [];
if(obj.inTable && source[0] == 'columns') {
if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) {
let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value));
currColumns = _.filter(currColumns, (cc)=>cc.local_column != oldColumn.name);
let column = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value));
currColumns = _.filter(currColumns, (cc)=>cc.column_cid != column.cid);
} else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) {
let tabColPath = _.slice(actionObj.path, 0, -1);
let oldColName = _.get(actionObj.oldState, tabColPath).name;
let idx = _.findIndex(currColumns, (cc)=>cc.local_column == oldColName);
let column = _.get(topState, tabColPath);
let idx = _.findIndex(currColumns, (cc)=>cc.column_cid == column.cid);
if(idx > -1) {
currColumns[idx].local_column = _.get(topState, tabColPath).name;
currColumns[idx].column = column.name;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class ForeignKeyHeaderSchema extends BaseUISchema {
let references_table_name = _.find(this.refTables, (t)=>t.value==data.references || t.value == this.origData.references)?.label;
return {
local_column: data.local_column,
local_column_cid: _.find(this.fieldOptions.local_column, (c)=>c.value == data.local_column)?.cid,
referenced: data.referenced,
references: data.references,
references_table_name: references_table_name,
Expand Down Expand Up @@ -346,12 +347,12 @@ export default class ForeignKeySchema extends BaseUISchema {
let currColumns = state.columns || [];
if(obj.inTable && source[0] == 'columns') {
if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) {
let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value));
currColumns = _.filter(currColumns, (cc)=>cc.local_column != oldColumn.name);
let column = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value));
currColumns = _.filter(currColumns, (cc)=>cc.local_column_cid != column.cid);
} else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) {
let tabColPath = _.slice(actionObj.path, 0, -1);
let oldColName = _.get(actionObj.oldState, tabColPath).name;
let idx = _.findIndex(currColumns, (cc)=>cc.local_column == oldColName);
let column = _.get(actionObj.oldState, tabColPath);
let idx = _.findIndex(currColumns, (cc)=>cc.local_column_cid == column.cid);
if(idx > -1) {
currColumns[idx].local_column = _.get(topState, tabColPath).name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ export default class PrimaryKeySchema extends BaseUISchema {
if(obj.inTable && source[0] == 'columns') {
if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) {
let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value));
currColumns = _.filter(currColumns, (cc)=>cc.column != oldColumn.name);
currColumns = _.filter(currColumns, (cc)=>cc.cid != oldColumn.cid);
} else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) {
let tabColPath = _.slice(actionObj.path, 0, -1);
let oldColName = _.get(actionObj.oldState, tabColPath).name;
let idx = _.findIndex(currColumns, (cc)=>cc.column == oldColName);
let oldCol = _.get(actionObj.oldState, tabColPath);
let idx = _.findIndex(currColumns, (cc)=>cc.cid == oldCol.cid);
if(idx > -1) {
currColumns[idx].column = _.get(topState, tabColPath).name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ export default class UniqueConstraintSchema extends BaseUISchema {
if(obj.inTable && source[0] == 'columns') {
if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) {
let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value));
currColumns = _.filter(currColumns, (cc)=>cc.column != oldColumn.name);
currColumns = _.filter(currColumns, (cc)=>cc.cid != oldColumn.cid);
} else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) {
let tabColPath = _.slice(actionObj.path, 0, -1);
let oldColName = _.get(actionObj.oldState, tabColPath).name;
let idx = _.findIndex(currColumns, (cc)=>cc.column == oldColName);
let oldCol = _.get(actionObj.oldState, tabColPath);
let idx = _.findIndex(currColumns, (cc)=>cc.cid == oldCol.cid);
if(idx > -1) {
currColumns[idx].column = _.get(topState, tabColPath).name;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ export default class TableSchema extends BaseUISchema {
}

changeColumnOptions(columns) {
let colOptions = (columns||[]).map((c)=>({label: c.name, value: c.name, image:'icon-column'}));
let colOptions = (columns||[]).map((c)=>({label: c.name, value: c.name, image:'icon-column', cid:c.cid}));
this.constraintsObj.changeColumnOptions(colOptions);
this.partitionKeysObj.changeColumnOptions(colOptions);
this.partitionsObj.changeColumnOptions(colOptions);
Expand Down Expand Up @@ -682,11 +682,12 @@ export default class TableSchema extends BaseUISchema {
let currPk = state.primary_key[0];
/* If col is not PK, remove it */
if(!columnData.is_primary_key) {
currPk.columns = _.filter(currPk.columns, (c)=>c.column !== columnData.name);
currPk.columns = _.filter(currPk.columns, (c)=>c.cid !== columnData.cid);
} else {
currPk.columns = _.filter(currPk.columns, (c)=>c.column !== columnData.name);
currPk.columns = _.filter(currPk.columns, (c)=>c.cid !== columnData.cid);
currPk.columns.push({
column: columnData.name,
cid: columnData.cid,
});
}
/* Remove the PK if all columns not PK */
Expand All @@ -699,7 +700,7 @@ export default class TableSchema extends BaseUISchema {
/* Create PK if none */
return {primary_key: [
obj.constraintsObj.primaryKeyObj.getNewData({
columns: [{column: columnData.name}],
columns: [{column: columnData.name, cid: columnData.cid}],
})
]};
}
Expand Down
2 changes: 2 additions & 0 deletions web/pgadmin/static/js/SchemaView/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,7 @@ function SchemaDialogView({

useEffect(()=>{
/* If reset key changes, reset the form */
schema.initialise(schema.origData);
sessDispatch({
type: SCHEMA_STATE_ACTIONS.INIT,
payload: schema.origData,
Expand All @@ -626,6 +627,7 @@ function SchemaDialogView({
const resetIt = ()=>{
firstEleRef.current?.focus();
setFormResetKey((prev)=>prev+1);
schema.initialise(schema.origData);
sessDispatch({
type: SCHEMA_STATE_ACTIONS.INIT,
payload: schema.origData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ describe('ExclusionConstraintSchema', ()=>{
is_exp: true,
column: 'abc',
col_type: null,
column_cid: null,
});
});
});
Expand All @@ -125,14 +126,14 @@ describe('ExclusionConstraintSchema', ()=>{
});

it('depChange', ()=>{
let state = {columns: [{local_column: 'id'}]};
let state = {columns: [{column: 'id', column_cid: 'c123'}]};

schemaObj.top = new TableSchema({}, null);
expect(getFieldDepChange(schemaObj, 'columns')(state, ['columns', 0], null, {
type: SCHEMA_STATE_ACTIONS.DELETE_ROW,
oldState: {
columns: [
{name: 'id'}
{name: 'id', cid: 'c123'}
],
},
path: ['columns'],
Expand All @@ -143,19 +144,19 @@ describe('ExclusionConstraintSchema', ()=>{

expect(getFieldDepChange(schemaObj, 'columns')(state, ['columns', 0], {
columns: [
{name: 'id123'}
{name: 'id123', cid: 'c123'}
],
}, {
type: SCHEMA_STATE_ACTIONS.SET_VALUE,
oldState: {
columns: [
{name: 'id'}
{name: 'id', cid: 'c123'}
],
},
path: ['columns', 0, 'name'],
value: 'id123',
})).toEqual({
columns: [{local_column: 'id123'}],
columns: [{column: 'id123', column_cid: 'c123'}],
});

state = {};
Expand Down

0 comments on commit 7374997

Please sign in to comment.