From 57ad91ffc0c5659beabbb069ef3be499bc1c133c Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 13:38:56 +0100 Subject: [PATCH 1/3] fix: keep inherited table columns read-only (#10179) Backend properties fetch marks a column already inherited from a parent table with inheritedfromtable, while a column fetched interactively via 'Inherited from table(s)' carries inheritedfrom instead and has no attnum yet. inSchemaWithColumnCheck only checked inheritedfrom, and did so after an isNew() short-circuit that treated the attnum-less interactive rows as new, so inherited columns ended up editable and deletable in both cases. Check both fields, and check them before the isNew() short-circuit, and extend canEditDeleteRowColumns the same way so the row's edit/delete buttons are disabled too. --- .../tables/columns/static/js/column.ui.js | 21 ++++++++++++------- .../schemas/tables/static/js/table.ui.js | 7 ++++++- .../schema_ui_files/column.ui.spec.js | 19 +++++++++++++++++ .../schema_ui_files/table.ui.spec.js | 6 ++++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js index 062464c504f..15f11894898 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js @@ -88,15 +88,22 @@ export default class ColumnSchema extends BaseUISchema { } if(this.nodeInfo && ('schema' in this.nodeInfo)) { - if(this.isNew(state)) { - return false; + // inheritedfrom/inheritedfromtable check is useful when we use this + // schema in table node. A column inherited from a parent table should + // always be read-only, whether it was already present when the table + // was opened (inheritedfromtable, set on the properties fetch) or was + // just added interactively via 'Inherited from table(s)' + // (inheritedfrom, set on the freshly fetched column). This must be + // checked before the isNew() check below, as interactively added + // inherited columns don't carry an attnum yet and would otherwise be + // (wrongly) treated as new, editable rows. + if (!isEmptyString(state.inheritedfrom) || + !isEmptyString(state.inheritedfromtable)){ + return true; } - // We will disable control if it's system columns - // inheritedfrom check is useful when we use this schema in table node - // inheritedfrom has value then we should disable it - if (!isEmptyString(state.inheritedfrom)){ - return true; + if(this.isNew(state)) { + return false; } // ie: it's position is less than 1 diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js index 4cc2bc46b47..6efb0064ca4 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js @@ -510,7 +510,12 @@ export default class TableSchema extends BaseUISchema { // Check for column grid when to edit/delete (for each row) canEditDeleteRowColumns(colstate) { - return isEmptyString(colstate.inheritedfrom); + // 'inheritedfrom' is set on columns fetched interactively via + // 'Inherited from table(s)'; 'inheritedfromtable' is set on columns + // already inherited when the table's properties were fetched. Both + // must disable the row's edit/delete buttons. + return isEmptyString(colstate.inheritedfrom) && + isEmptyString(colstate.inheritedfromtable); } isPartitioned(state) { diff --git a/web/regression/javascript/schema_ui_files/column.ui.spec.js b/web/regression/javascript/schema_ui_files/column.ui.spec.js index cc99104e044..a7ef03db946 100644 --- a/web/regression/javascript/schema_ui_files/column.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/column.ui.spec.js @@ -133,6 +133,25 @@ describe('ColumnSchema', ()=>{ expect(schemaObj.inSchemaWithColumnCheck(state)).toBe(false); }); + it('inSchemaWithColumnCheck - column already inherited from a parent table', ()=>{ + // Set on the properties fetch for a column the table already + // inherits when opened (issue #10179, case 1). + schemaObj.nodeInfo = {schema: {}}; + let state = {attnum: 1, inheritedfromtable: 'public.parent'}; + expect(schemaObj.inSchemaWithColumnCheck(state)).toBe(true); + expect(schemaObj.editableCheckForTable(state)).toBe(false); + }); + + it('inSchemaWithColumnCheck - column added interactively via Inherited from table(s)', ()=>{ + // Columns freshly fetched via 'Inherited from table(s)' don't carry an + // attnum yet, so isNew() would otherwise (wrongly) treat them as new, + // editable rows (issue #10179, case 2). + schemaObj.nodeInfo = {schema: {}}; + let state = {name: 'id', inheritedfrom: 'public.parent'}; + expect(schemaObj.inSchemaWithColumnCheck(state)).toBe(true); + expect(schemaObj.editableCheckForTable(state)).toBe(false); + }); + it('editableCheckForTable', ()=>{ let state = {}; schemaObj.nodeInfo = {}; diff --git a/web/regression/javascript/schema_ui_files/table.ui.spec.js b/web/regression/javascript/schema_ui_files/table.ui.spec.js index 46fa0b1d67c..7482debdaad 100644 --- a/web/regression/javascript/schema_ui_files/table.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/table.ui.spec.js @@ -73,6 +73,12 @@ describe('TableSchema', () => { it('canEditDeleteRowColumns', () => { expect(schemaObj.canEditDeleteRowColumns({inheritedfrom: 1234})).toBe(false); expect(schemaObj.canEditDeleteRowColumns({inheritedfrom: null})).toBe(true); + + // Column already inherited from a parent table when the table was + // opened (issue #10179, case 1) - the row's edit/delete buttons must + // be disabled too. + expect(schemaObj.canEditDeleteRowColumns({inheritedfromtable: 'public.parent'})).toBe(false); + expect(schemaObj.canEditDeleteRowColumns({inheritedfromtable: null})).toBe(true); }); it('LikeSchema typname change', () => { From da8628df6c04c57179c55e567ab490e08feda049 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 13:39:29 +0100 Subject: [PATCH 2/3] fix: restrict Definition-tab Data type options to edit_types (#10180) The expanded row's Definition tab and the inline grid-cell editor each defined their own copy of the edit_types filter for the 'cltype' field, but the tab's version received the whole table's data as 'state' rather than the row, since MappedControl resolves a field's 'type' callback against the top-level schema, not the field's own row (unlike 'cell', which already gets the full row). That made isNew() and edit_types resolve against the wrong object, so the filter always no-opped and the tab showed every type instead of the restricted set. Have MappedControl also resolve a field's declared 'deps' against its own row (listenDepChanges already does this correctly for 'cell') and forward them as a 2nd argument to 'type', mirroring what 'cell' already receives. column.ui.js declares 'edit_types'/'attnum' as deps on 'cltype' and factors the filter into one shared editTypesFilter() used by both 'cell' and 'type', so the two stay in sync by construction. --- .../tables/columns/static/js/column.ui.js | 44 +++++++++------- .../static/js/SchemaView/MappedControl.jsx | 10 +++- .../schema_ui_files/column.ui.spec.js | 51 +++++++++++++++++++ 3 files changed, 87 insertions(+), 18 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js index 15f11894898..5121dd9d50f 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js @@ -171,6 +171,22 @@ export default class ColumnSchema extends BaseUISchema { return !isEmptyString(state.inheritedfromtype); } + // Shared by the inline grid-cell 'Data type' editor and the expanded + // Definition tab's 'Data type' dropdown, so both apply the exact same + // edit_types restriction for the same column. isRowNew must be computed + // by the caller against the *row's* own state (not the enclosing table's + // or the field's own scalar state), since new columns can be set to any + // type whilst existing ones may only be altered to one of edit_types. + editTypesFilter(edit_types, isRowNew) { + return (options)=>{ + if (isRowNew || this.inErd) { + return options; + } + let allowed = edit_types || []; + return _.filter(options, (o)=>allowed.indexOf(o.value) > -1); + }; + } + get baseFields() { let obj = this; @@ -248,20 +264,21 @@ export default class ColumnSchema extends BaseUISchema { group: gettext('Definition'), noEmpty: true, editable: this.editableCheckForTable, options: this.cltypeOptions, optionsLoaded: (options)=>{obj.datatypes = options;}, - type: (state)=>{ + // 'edit_types'/'attnum' are declared as deps purely so that the + // schema view resolves them against this row (not the whole table), + // and passes them through as the 2nd (depVals) argument below. This + // is what lets the expanded Definition tab's dropdown apply the same + // edit_types restriction as the inline grid-cell editor, whose + // 'cell' callback already receives the full row. + deps: ['edit_types', 'attnum'], + type: (state, depVals)=>{ + let [edit_types, attnum] = depVals || []; return { type: 'select', options: this.cltypeOptions, controlProps: { allowClear: false, - filter: (options)=>{ - let result = options; - let edit_types = state?.edit_types || []; - if(!obj.isNew(state) && !this.inErd) { - result = _.filter(options, (o)=>edit_types.indexOf(o.value) > -1); - } - return result; - }, + filter: obj.editTypesFilter(edit_types, obj.isNew({attnum})), } }; }, @@ -271,14 +288,7 @@ export default class ColumnSchema extends BaseUISchema { options: this.cltypeOptions, controlProps: { allowClear: false, - filter: (options)=>{ - let result = options; - let edit_types = row?.edit_types || []; - if(!obj.isNew(row) && !this.inErd) { - result = _.filter(options, (o)=>edit_types.indexOf(o.value) > -1); - } - return result; - }, + filter: obj.editTypesFilter(row?.edit_types, obj.isNew(row)), } }; } diff --git a/web/pgadmin/static/js/SchemaView/MappedControl.jsx b/web/pgadmin/static/js/SchemaView/MappedControl.jsx index 78353a07097..0adfb0d4285 100644 --- a/web/pgadmin/static/js/SchemaView/MappedControl.jsx +++ b/web/pgadmin/static/js/SchemaView/MappedControl.jsx @@ -401,7 +401,15 @@ export const MappedFormControl = ({ } if (typeof (field.type) === 'function') { - const typeProps = evalFunc(null, field.type, state); + // 'state' here is the whole top-level schema data, not this field's + // row, since a field nested inside a collection row shares the same + // accessPath resolution as any other field. 'depVals' (already resolved + // against this field's own row via 'deps', see listenDepChanges above) + // is passed as a 2nd argument so a field.type() callback can access + // sibling fields from its own row, mirroring what field.cell() already + // gets via its row argument. Existing field.type() callbacks that only + // take a single argument are unaffected. + const typeProps = evalFunc(null, field.type, state, depVals); newProps = { ...newProps, ...typeProps, diff --git a/web/regression/javascript/schema_ui_files/column.ui.spec.js b/web/regression/javascript/schema_ui_files/column.ui.spec.js index a7ef03db946..66dfd642d77 100644 --- a/web/regression/javascript/schema_ui_files/column.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/column.ui.spec.js @@ -152,6 +152,57 @@ describe('ColumnSchema', ()=>{ expect(schemaObj.editableCheckForTable(state)).toBe(false); }); + it('editTypesFilter', ()=>{ + let options = [ + {label: 'integer', value: 'integer'}, + {label: 'text', value: 'text'}, + {label: 'boolean', value: 'boolean'}, + ]; + + // Existing column: restricted to edit_types. + let filtered = schemaObj.editTypesFilter(['integer', 'text'], false)(options); + expect(filtered).toEqual([ + {label: 'integer', value: 'integer'}, + {label: 'text', value: 'text'}, + ]); + + // New column: unrestricted, full list. + expect(schemaObj.editTypesFilter(['integer'], true)(options)).toEqual(options); + + // No edit_types available: restricts down to nothing. + expect(schemaObj.editTypesFilter(undefined, false)(options)).toEqual([]); + + // ERD is always unrestricted, regardless of edit_types/isNew. + schemaObj.inErd = true; + expect(schemaObj.editTypesFilter(['integer'], false)(options)).toEqual(options); + schemaObj.inErd = false; + }); + + it('cltype - expanded Definition tab options match the inline grid-cell options', ()=>{ + let cltypeField = _.find(schemaObj.fields, (f)=>f.id === 'cltype'); + let options = [ + {label: 'integer', value: 'integer'}, + {label: 'text', value: 'text'}, + {label: 'boolean', value: 'boolean'}, + ]; + let row = {attnum: 1, edit_types: ['integer', 'boolean']}; + + // Inline grid-cell editor. + let cellResult = cltypeField.cell(row); + let cellFiltered = cellResult.controlProps.filter(options); + + // Expanded Definition tab, as it is invoked once 'edit_types'/'attnum' + // have been resolved against this row via deps (see MappedControl.jsx). + let typeResult = cltypeField.type(row.cltype, [row.edit_types, row.attnum]); + let typeFiltered = typeResult.controlProps.filter(options); + + expect(typeFiltered).toEqual(cellFiltered); + expect(typeFiltered).toEqual([ + {label: 'integer', value: 'integer'}, + {label: 'boolean', value: 'boolean'}, + ]); + }); + it('editableCheckForTable', ()=>{ let state = {}; schemaObj.nodeInfo = {}; From 4222d96589548dcf4da86ecb0ab8a38ec5417eb5 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 25 Aug 2026 09:59:01 +0100 Subject: [PATCH 3/3] fix: honour inheritedfromtable in attlen/attprecision editable checks The Length/Precision and Scale grid-cell editors only checked inheritedfrom when deciding whether to allow editing, so a column loaded with only inheritedfromtable set (i.e. already inherited when the table was opened) could be left incorrectly editable, unlike the other inherited-column checks in this schema. Add a shared isInheritedColumn() helper covering both fields and reuse it in both editable callbacks and in inSchemaWithColumnCheck. --- .../tables/columns/static/js/column.ui.js | 24 +++++++++++++------ .../schema_ui_files/column.ui.spec.js | 21 ++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js index 5121dd9d50f..dc6958a128d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js @@ -97,8 +97,7 @@ export default class ColumnSchema extends BaseUISchema { // checked before the isNew() check below, as interactively added // inherited columns don't carry an attnum yet and would otherwise be // (wrongly) treated as new, editable rows. - if (!isEmptyString(state.inheritedfrom) || - !isEmptyString(state.inheritedfromtable)){ + if (this.isInheritedColumn(state)) { return true; } @@ -171,6 +170,17 @@ export default class ColumnSchema extends BaseUISchema { return !isEmptyString(state.inheritedfromtype); } + // A column inherited from a parent table should be treated as read-only, + // whether it was already present when the table was opened + // (inheritedfromtable, set on the properties fetch) or was just added + // interactively via 'Inherited from table(s)' (inheritedfrom, set on the + // freshly fetched column). Shared by any editable/disabled check that + // needs to special-case inherited columns. + isInheritedColumn(state) { + return !isEmptyString(state.inheritedfrom) || + !isEmptyString(state.inheritedfromtable); + } + // Shared by the inline grid-cell 'Data type' editor and the expanded // Definition tab's 'Data type' dropdown, so both apply the exact same // edit_types restriction for the same column. isRowNew must be computed @@ -387,8 +397,8 @@ export default class ColumnSchema extends BaseUISchema { return !obj.attlenRange(state); }, editable: function(state) { - // inheritedfrom has value then we should disable it - if (!isEmptyString(state.inheritedfrom)) { + // A column inherited from a parent table should stay read-only. + if (obj.isInheritedColumn(state)) { return false; } return Boolean(obj.attlenRange(state)); @@ -419,11 +429,11 @@ export default class ColumnSchema extends BaseUISchema { return !this.attprecisionRange(state); }, editable: function(state) { - // inheritedfrom has value then we should disable it - if (!isEmptyString(state.inheritedfrom)) { + // A column inherited from a parent table should stay read-only. + if (obj.isInheritedColumn(state)) { return false; } - return Boolean(this.attprecisionRange(state)); + return Boolean(obj.attprecisionRange(state)); }, },{ id: 'min_val_attprecision', skipChange: true, visible: false, type: '', diff --git a/web/regression/javascript/schema_ui_files/column.ui.spec.js b/web/regression/javascript/schema_ui_files/column.ui.spec.js index 66dfd642d77..ce31215b909 100644 --- a/web/regression/javascript/schema_ui_files/column.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/column.ui.spec.js @@ -152,6 +152,27 @@ describe('ColumnSchema', ()=>{ expect(schemaObj.editableCheckForTable(state)).toBe(false); }); + it('attlen/attprecision editable - column already inherited from a parent table (inheritedfromtable only)', ()=>{ + // A loaded row can carry only inheritedfromtable, with no + // inheritedfrom (issue #10179, case 1). The attlen/attprecision + // editable checks must honour that case too, not just the + // interactively-added 'inheritedfrom' case. + schemaObj.datatypes = datatypes; + let attlenField = _.find(schemaObj.fields, (f)=>f.id === 'attlen'); + let attprecisionField = _.find(schemaObj.fields, (f)=>f.id === 'attprecision'); + + let varcharState = {cltype: 'character varying', inheritedfromtable: 'public.parent'}; + expect(attlenField.editable(varcharState)).toBe(false); + + let numericState = {cltype: 'numeric', inheritedfromtable: 'public.parent'}; + expect(attprecisionField.editable(numericState)).toBe(false); + + // Sanity check: without inheritance, they remain editable when the + // data type supports length/precision. + expect(attlenField.editable({cltype: 'character varying'})).toBe(true); + expect(attprecisionField.editable({cltype: 'numeric'})).toBe(true); + }); + it('editTypesFilter', ()=>{ let options = [ {label: 'integer', value: 'integer'},