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.

Sign up for GitHub

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

Added the ability to change sql security option without specifying the entire body of the function/procedure/package #7807

Merged
merged 6 commits into from
Nov 10, 2023
+252 −53
Merged

Added the ability to change sql security option without specifying the entire body of the function/procedure/package #7807

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9b823e6
Add ability to change sql security option in alter function
Oct 16, 2023
5b62093
added the ability to change sql security opt in alter procedure
Oct 25, 2023
695082e
added the ability to change sql security option in alter package
Oct 25, 2023
b774645
refactoring rules for partial alters
Oct 30, 2023
93c1b1f
Merge branch 'master' into master
Zhdanov0 Nov 10, 2023
ecc0fec
triState namespace fix
Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions doc/sql.extensions/README.ddl.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,17 @@ ALTER TABLE <name> ... [ {ENABLE | DISABLE} PUBLICATION ]
Defines whether replication is enabled for the specified table.
If not specified in the CREATE TABLE statement, the database-level default behaviour is applied.

24) Added the ability to change deterministic option without specifying the entire body of the function.
24) Added the ability to change deterministic and sql security option without specifying the entire body of the function.
(Alexander Zhdanov)

ALTER FUNCTION <name> {DETERMINISTIC | NOT DETERMINISTIC}
ALTER FUNCTION <name> [ {DETERMINISTIC | NOT DETERMINISTIC} ] [ SQL SECURITY {DEFINER | INVOKER} | DROP SQL SECURITY ]

25) Added the ability to change sql security option without specifying the entire body of the procedure
(Alexander Zhdanov)

ALTER PROCEDURE <name> SQL SECURITY {DEFINER | INVOKER} | DROP SQL SECURITY

26) Added the ability to change sql security option without specifying the entire body of the package
(Alexander Zhdanov)

ALTER PACKAGE <name> SQL SECURITY {DEFINER | INVOKER} | DROP SQL SECURITY
102 changes: 88 additions & 14 deletions src/dsql/DdlNodes.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,7 @@ DdlNode* CreateAlterFunctionNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
returnType->type->resolve(dsqlScratch);

// check SQL SECURITY is not set if function declared in package
if (package.hasData() && ssDefiner.isAssigned())
if (package.hasData() && ssDefiner.has_value())
{
ERRD_post(Arg::Gds(isc_sqlerr) << Arg::Num(-204) <<
Arg::Gds(isc_invalid_clause) << Arg::Str("SQL SECURITY for functions is prohibit in packages"));
Expand Down Expand Up @@ -1905,13 +1905,13 @@ bool CreateAlterFunctionNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch*
attachment->storeMetaDataBlob(tdbb, transaction, &FUN.RDB$FUNCTION_SOURCE, source);

FUN.RDB$DETERMINISTIC_FLAG.NULL = FALSE;
FUN.RDB$DETERMINISTIC_FLAG = deterministic ? TRUE : FALSE;
FUN.RDB$DETERMINISTIC_FLAG = deterministic.asBool() ? TRUE : FALSE;
FUN.RDB$RETURN_ARGUMENT = 0;

if (ssDefiner.isAssigned())
if (ssDefiner.has_value())
{
FUN.RDB$SQL_SECURITY.NULL = FALSE;
FUN.RDB$SQL_SECURITY = ssDefiner.asBool() ? FB_TRUE : FB_FALSE;
FUN.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
FUN.RDB$SQL_SECURITY.NULL = TRUE;
Expand Down Expand Up @@ -2101,8 +2101,21 @@ bool CreateAlterFunctionNode::executeAlterIndividualParameters(thread_db* tdbb,
}

MODIFY FUN
FUN.RDB$DETERMINISTIC_FLAG.NULL = FALSE;
FUN.RDB$DETERMINISTIC_FLAG = deterministic ? TRUE : FALSE;
if(deterministic.isAssigned())
{
FUN.RDB$DETERMINISTIC_FLAG.NULL = FALSE;
FUN.RDB$DETERMINISTIC_FLAG = deterministic.asBool() ? TRUE : FALSE;
}
if (ssDefiner.has_value())
{
if(ssDefiner.value() != SqlSecurity::SS_DROP)
{
FUN.RDB$SQL_SECURITY.NULL = FALSE;
FUN.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
FUN.RDB$SQL_SECURITY.NULL = TRUE;
}

modifed = true;
END_MODIFY
Expand Down Expand Up @@ -2698,7 +2711,7 @@ DdlNode* CreateAlterProcedureNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
returns[i]->type->resolve(dsqlScratch);

// check SQL SECURITY is not set if procedure declared in package
if (package.hasData() && ssDefiner.isAssigned())
if (package.hasData() && ssDefiner.has_value())
{
ERRD_post(Arg::Gds(isc_sqlerr) << Arg::Num(-204) <<
Arg::Gds(isc_invalid_clause) << Arg::Str("SQL SECURITY for procedures is prohibit in packages"));
Expand Down Expand Up @@ -2729,11 +2742,22 @@ void CreateAlterProcedureNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsq
AutoSavePoint savePoint(tdbb, transaction);
bool altered = false;

const bool alterIndividualParameters = (alter && !(body || external));

// first pass
if (alter)
if (alterIndividualParameters)
{
if (executeAlterIndividualParameters(tdbb, dsqlScratch, transaction, false, true))
altered = true;
else
status_exception::raise(Arg::Gds(isc_dyn_proc_not_found) << Arg::Str(name));
}
else if (alter)
{
if (executeAlter(tdbb, dsqlScratch, transaction, false, true))
{
altered = true;
}
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
{
altered = true;
}
altered = true;

else
{
if (create) // create or alter
Expand All @@ -2747,7 +2771,11 @@ void CreateAlterProcedureNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsq

compile(tdbb, dsqlScratch);

executeAlter(tdbb, dsqlScratch, transaction, true, false); // second pass
// second pass
if (alterIndividualParameters)
executeAlterIndividualParameters(tdbb, dsqlScratch, transaction, true, false);
else
executeAlter(tdbb, dsqlScratch, transaction, true, false);

if (package.isEmpty())
{
Expand Down Expand Up @@ -2896,10 +2924,10 @@ bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch
else
P.RDB$PRIVATE_FLAG.NULL = TRUE;

if (ssDefiner.isAssigned())
if (ssDefiner.has_value())
{
P.RDB$SQL_SECURITY.NULL = FALSE;
P.RDB$SQL_SECURITY = ssDefiner.asBool() ? FB_TRUE : FB_FALSE;
P.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
P.RDB$SQL_SECURITY.NULL = TRUE;
Expand Down Expand Up @@ -3011,6 +3039,52 @@ bool CreateAlterProcedureNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch
return modified;
}

bool CreateAlterProcedureNode::executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction, bool secondPass, bool runTriggers)
{
Attachment* const attachment = transaction->getAttachment();

bool modifed = false;

AutoCacheRequest requestHandle(tdbb, drq_m_prm_prcs2, DYN_REQUESTS);

FOR (REQUEST_HANDLE requestHandle TRANSACTION_HANDLE transaction)
P IN RDB$PROCEDURES
WITH P.RDB$PROCEDURE_NAME EQ name.c_str() AND
P.RDB$PACKAGE_NAME EQUIV NULLIF(package.c_str(), '')
{
if (P.RDB$SYSTEM_FLAG)
{
status_exception::raise(
Arg::Gds(isc_dyn_cannot_mod_sysproc) <<
MetaName(P.RDB$PROCEDURE_NAME));
}

if (!secondPass && runTriggers && package.isEmpty())
{
executeDdlTrigger(tdbb, dsqlScratch, transaction, DTW_BEFORE,
DDL_TRIGGER_ALTER_PROCEDURE, name, NULL);
}

MODIFY P
if (ssDefiner.has_value())
{
if(ssDefiner.value() != SqlSecurity::SS_DROP)
{
P.RDB$SQL_SECURITY.NULL = FALSE;
P.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
P.RDB$SQL_SECURITY.NULL = TRUE;
}

modifed = true;
END_MODIFY
}
END_FOR

return modifed;
}

void CreateAlterProcedureNode::storeParameter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch,
jrd_tra* transaction, USHORT parameterType, unsigned pos, ParameterClause* parameter,
const CollectedParameter* collectedParameter)
Expand Down Expand Up @@ -3516,10 +3590,10 @@ bool TriggerDefinition::modify(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch

if (ssDefiner.has_value())
{
if (ssDefiner.value() != SS_DROP)
if (ssDefiner.value() != SqlSecurity::SS_DROP)
{
TRG.RDB$SQL_SECURITY.NULL = FALSE;
TRG.RDB$SQL_SECURITY = ssDefiner.value() == SS_DEFINER ? FB_TRUE : FB_FALSE;
TRG.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
TRG.RDB$SQL_SECURITY.NULL = TRUE;
Expand Down Expand Up @@ -3573,7 +3647,7 @@ DdlNode* CreateAlterTriggerNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
}
}

if (create && ssDefiner.has_value() && ssDefiner.value() == TriggerDefinition::SS_DROP)
if (create && ssDefiner.has_value() && ssDefiner.value() == SqlSecurity::SS_DROP)
status_exception::raise(Arg::Gds(isc_dsql_command_err) << Arg::Gds(isc_dsql_invalid_drop_ss_clause));

return DdlNode::dsqlPass(dsqlScratch);
Expand Down
23 changes: 13 additions & 10 deletions src/dsql/DdlNodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@

namespace Jrd {

enum SqlSecurity
{
SS_INVOKER,
SS_DEFINER,
SS_DROP
};

class LocalDeclarationsNode;
class RelationSourceNode;
class ValueListNode;
Expand Down Expand Up @@ -414,7 +421,6 @@ class CreateAlterFunctionNode : public DdlNode
create(true),
alter(false),
external(NULL),
deterministic(false),
parameters(pool),
returnType(NULL),
localDeclList(NULL),
Expand Down Expand Up @@ -469,7 +475,7 @@ class CreateAlterFunctionNode : public DdlNode
bool create;
bool alter;
NestConst<ExternalClause> external;
bool deterministic;
TriState deterministic;
Firebird::Array<NestConst<ParameterClause> > parameters;
NestConst<ParameterClause> returnType;
NestConst<LocalDeclarationsNode> localDeclList;
Expand All @@ -482,7 +488,7 @@ class CreateAlterFunctionNode : public DdlNode
bool privateScope;
bool preserveDefaults;
SLONG udfReturnPos;
TriState ssDefiner;
std::optional<SqlSecurity> ssDefiner;
};


Expand Down Expand Up @@ -594,6 +600,9 @@ class CreateAlterProcedureNode : public DdlNode
void executeCreate(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);
bool executeAlter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
bool secondPass, bool runTriggers);
bool executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
bool secondPass, bool runTriggers);

void storeParameter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction,
USHORT parameterType, unsigned pos, ParameterClause* parameter,
const CollectedParameter* collectedParameter);
Expand All @@ -616,7 +625,7 @@ class CreateAlterProcedureNode : public DdlNode
MetaName packageOwner;
bool privateScope;
bool preserveDefaults;
TriState ssDefiner;
std::optional<SqlSecurity> ssDefiner;
};


Expand Down Expand Up @@ -661,12 +670,6 @@ typedef RecreateNode<CreateAlterProcedureNode, DropProcedureNode, isc_dsql_recre
class TriggerDefinition
{
public:
enum SqlSecurity
{
SS_INVOKER,
SS_DEFINER,
SS_DROP
};

explicit TriggerDefinition(MemoryPool& p)
: name(p),
Expand Down
60 changes: 55 additions & 5 deletions src/dsql/PackageNodes.epp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ string CreateAlterPackageNode::internalPrint(NodePrinter& printer) const

DdlNode* CreateAlterPackageNode::dsqlPass(DsqlCompilerScratch* dsqlScratch)
{
if(alter && !items)
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(alter && !items)
if (alter && !items)

return DdlNode::dsqlPass(dsqlScratch);

MemoryPool& pool = dsqlScratch->getPool();

source.ltrim("\n\r\t ");
Expand Down Expand Up @@ -319,9 +322,18 @@ void CreateAlterPackageNode::execute(thread_db* tdbb, DsqlCompilerScratch* dsqlS
// run all statements under savepoint control
AutoSavePoint savePoint(tdbb, transaction);

const bool alterIndividualParameters = (alter && !items);

if (alter)
{
if (!executeAlter(tdbb, dsqlScratch, transaction))
if(alterIndividualParameters)
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(alterIndividualParameters)
if (alterIndividualParameters)

{
if(!executeAlterIndividualParameters(tdbb, dsqlScratch, transaction))
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(!executeAlterIndividualParameters(tdbb, dsqlScratch, transaction))
if (!executeAlterIndividualParameters(tdbb, dsqlScratch, transaction))

status_exception::raise(
Arg::Gds(isc_no_meta_update) <<
Arg::Gds(isc_dyn_package_not_found) << Arg::Str(name));
}
else if (!executeAlter(tdbb, dsqlScratch, transaction))
{
if (create) // create or alter
executeCreate(tdbb, dsqlScratch, transaction);
Expand Down Expand Up @@ -371,10 +383,10 @@ void CreateAlterPackageNode::executeCreate(thread_db* tdbb, DsqlCompilerScratch*
PKG.RDB$PACKAGE_HEADER_SOURCE.NULL = FALSE;
attachment->storeMetaDataBlob(tdbb, transaction, &PKG.RDB$PACKAGE_HEADER_SOURCE, source);

if (ssDefiner.isAssigned())
if (ssDefiner.has_value())
{
PKG.RDB$SQL_SECURITY.NULL = FALSE;
PKG.RDB$SQL_SECURITY = ssDefiner.asBool() ? FB_TRUE : FB_FALSE;
PKG.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
PKG.RDB$SQL_SECURITY.NULL = TRUE;
Expand Down Expand Up @@ -445,10 +457,10 @@ bool CreateAlterPackageNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch*
if (!PKG.RDB$VALID_BODY_FLAG.NULL)
PKG.RDB$VALID_BODY_FLAG = FALSE;

if (ssDefiner.isAssigned())
if (ssDefiner.has_value())
{
PKG.RDB$SQL_SECURITY.NULL = FALSE;
PKG.RDB$SQL_SECURITY = ssDefiner.asBool() ? FB_TRUE : FB_FALSE;
PKG.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
PKG.RDB$SQL_SECURITY.NULL = TRUE;
Expand All @@ -475,6 +487,44 @@ bool CreateAlterPackageNode::executeAlter(thread_db* tdbb, DsqlCompilerScratch*
}


bool CreateAlterPackageNode::executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction)
{
AutoCacheRequest requestHandle(tdbb, drq_m_prm_pkg, DYN_REQUESTS);
bool modified = false;

FOR (REQUEST_HANDLE requestHandle TRANSACTION_HANDLE transaction)
PKG IN RDB$PACKAGES
WITH PKG.RDB$PACKAGE_NAME EQ name.c_str()
{
modified = true;

executeDdlTrigger(tdbb, dsqlScratch, transaction, DTW_BEFORE,
DDL_TRIGGER_ALTER_PACKAGE, name, NULL);

MODIFY PKG
if (ssDefiner.has_value())
{
if(ssDefiner.value() != SqlSecurity::SS_DROP)
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(ssDefiner.value() != SqlSecurity::SS_DROP)
if (ssDefiner.value() != SqlSecurity::SS_DROP)

{
PKG.RDB$SQL_SECURITY.NULL = FALSE;
PKG.RDB$SQL_SECURITY = ssDefiner.value() == SqlSecurity::SS_DEFINER ? FB_TRUE : FB_FALSE;
}
else
PKG.RDB$SQL_SECURITY.NULL = TRUE;
}
END_MODIFY
}
END_FOR

if (modified)
{
executeDdlTrigger(tdbb, dsqlScratch, transaction,
DTW_AFTER, DDL_TRIGGER_ALTER_PACKAGE, name, NULL);
}

return modified;
}

void CreateAlterPackageNode::executeItems(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch,
jrd_tra* transaction)
{
Expand Down
3 changes: 2 additions & 1 deletion src/dsql/PackageNodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class CreateAlterPackageNode : public DdlNode
private:
void executeCreate(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);
bool executeAlter(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);
bool executeAlterIndividualParameters(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);
void executeItems(thread_db* tdbb, DsqlCompilerScratch* dsqlScratch, jrd_tra* transaction);

public:
Expand All @@ -110,7 +111,7 @@ class CreateAlterPackageNode : public DdlNode
Firebird::Array<Item>* items;
Firebird::SortedArray<MetaName> functionNames;
Firebird::SortedArray<MetaName> procedureNames;
TriState ssDefiner;
std::optional<SqlSecurity> ssDefiner;

private:
MetaName owner;
Expand Down
Loading