Allow to create database with different owner

[aafemt]

No description provided.

[AlexPeshkoff]

Feature is for sure useful, and I finally agree with removing parameter from getUserInfo() as related with this PR - but please remove unrelated changes from .pas file.

[hvlad]

It is always good idea to explain why you consider proposed feature useful and how it could/should be used.

[aafemt]

please remove unrelated changes from .pas file

Ok, but should I really edit autogenerated file by hand after every build?

[aafemt]

why you consider proposed feature useful and how it could/should be used.

Among other purposes it is planned to be used as a base for other feature: gbak preserving DB owner on restore. It would bring consistency between nbackup preserving owner and gbak rewriting it.

[AlexPeshkoff]

Agreed, this change is related.

[AlexPeshkoff]

Ok, but should I really edit autogenerated file by hand after every build?

Certainly not. I will fix file in master branch and after it pls fix PR.

[aafemt]

That's why I insisted that this file must not be in the repository years ago...

[AlexPeshkoff]

@aafemt Look like you've created PR against rather old copy of this file. If you make fresh clone of master branch and run something like:
git log -L 14927,14935:Firebird.pas
you will see why I think so.
I do not pretend to understand how exactly you've achieved this :)

[aafemt]

Sorry, I don't understand this as well. I've forked the branch from master after "git pull".

Perhaps, CLOOP works differently on Linux and Windows.

[AlexPeshkoff]

Last fix was done by Vlad and I doubt he used linux build for that :-)
Well , I suggest to treat this incident as resolved :)

What about PR as a whole - let's wait for DE cause list of features to be added to FB5 was already discussed and currently is closed. Personally I see no problems adding it - your code appears quite clear here, but let's wait what others say.

[aafemt]

Last fix was done by Vlad and I doubt he used linux build for that :-)

But you are looking at different line. Here is link to current master version:

firebird/src/include/gen/Firebird.pas

Line 15197 in 5432699

function ITraceProcedureImpl_getStmtIDDispatcher(this: ITraceProcedure): Int64; cdecl;

[aafemt]

Personally I see no problems adding it - your code appears quite clear here, but let's wait what others say.

I don't expect this PR to be accepted into v5. V6 is fine. I just need to reserve DPB item number.

PS: More controversial changes will follow. That's why the PR is marked as a draft.

[sim1984]

I did not see support for specifying the owner of the database in the create database statement. It is clear that this pseudo-statement in reality calls the api to create a database, but it also needs to be done. If it is, then why is it not reflected in the documentation?

[aafemt]

That will be in the next commit(s) as mentioned above.

[AlexPeshkoff]

On 8/23/23 18:15, Dimitry Sibiryakov wrote: Last fix was done by Vlad and I doubt he used linux build for that :-) But you are looking at different line. Here is link to current master version: https://github.com/FirebirdSQL/firebird/blob/5432699f284dad21443eff5ed86b9f717614fa6f/src/include/gen/Firebird.pas#L15197

Wrong link. Coorect one is https://github.com/FirebirdSQL/firebird/blob/master/src/include/gen/Firebird.pas#L14927 Your link is not to master. Instead 5432699 <https://github.com/FirebirdSQL/firebird/blob/5432699f284dad21443eff5ed86b9f717614fa6f/src/include/gen/Firebird.pas#L15197> is used.

[aafemt]

Wrong link. Coorect one is https://github.com/FirebirdSQL/firebird/blob/master/src/include/gen/Firebird.pas#L14927 Your link is not to master.

It was to the last relevant commit to master but ok, here is the really right link:
https://github.com/FirebirdSQL/firebird/blob/master/src/include/gen/Firebird.pas#L15197

[AlexPeshkoff]

On 8/23/23 18:49, Dimitry Sibiryakov wrote: Wrong link. Coorect one is https://github.com/FirebirdSQL/firebird/blob/master/src/include/gen/Firebird.pas#L14927 Your link is not to master. It is to the last commit of master but ok, here is the really right link: https://github.com/FirebirdSQL/firebird/blob/master/src/include/gen/Firebird.pas#L15197

You are right. A bug though not in cloop but in posix makefile. Will fix.

[AlexPeshkoff]

On 8/24/23 14:45, Dimitry Sibiryakov wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In src/jrd/DbCreators.h <#7718 (comment)>: > @@ -36,7 +36,14 @@ namespace Jrd { -bool checkCreateDatabaseGrant(const Firebird::MetaString& userName, const Firebird::MetaString& trustedRole, +enum class CreateGrant +{ + none, // This user cannot create databases Isn't it only for old code which was converted from macros?

No.

All-uppercase identifiers are ugly.

When they represent constants - may be not too ugly.

[aafemt]

When they represent constants - may be not too ugly.

Ok, though it is a scoped enum and its members cannot be mistaken for anything else.

[livius2]

Wouldn't it be better to make the options available
grant DATABASE_OWNER to USER_NAME?

[aafemt]

Wouldn't it be better to make the options available grant DATABASE_OWNER to USER_NAME?

Changing of existing database is problematic. Set owner during creation is simpler.

[pavel-zotov]

Is it possible to implement similar for ALTER database ? I mean:

alter database set owner to <some_other_user>;

[sim1984]

Implementing changing the owner of just the database will not solve the problems. By introducing this feature, you will also have to provide the ability to change owners for other database objects. And also think about recursively changing the owner of all database objects.

ALTER <object_type> <object_name> SET OWNER TO <user>

<object_type> ::=
  TABLE | VIEW | PROCEDURE | FUNCTION | TRIGGER | PACKAGE |
  DOMAIN | EXCEPTION | FILTER | ROLE

And special case for database:

ALTER DATABASE SET OWNER TO <user> [WITH RECURSIVE]

[pavel-zotov]

And also think about recursively changing the owner of all database objects.

Yes, number of these objects can be huge. But not infinite.
And not all of objects must be affected but only those who were created by previous owner.
So, several minutes usually must be enough to complete this job :-)

PS. Of course, i speak about 'service time' interval when SYSDBA has exclusive access to DB

[aafemt]

Is it possible to implement similar for ALTER database ?

In theory - yes. On practice it is much harder task than impersonation during single call.