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.

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

Ignore missing UDR libraries during restore #7168

Closed
dyemanov opened this issue Apr 8, 2022 · 5 comments
Closed

Ignore missing UDR libraries during restore #7168

dyemanov opened this issue Apr 8, 2022 · 5 comments

Comments

@dyemanov
Copy link
Member

dyemanov commented Apr 8, 2022

We have this supported for UDFs since the first FB versions and it's quite handy, especially for developers/supporters dealing with user databases. It's not working for UDRs though, error is thrown during restore ("UDR module not loaded"). It would be nice to be able to restore such databases, with nullified module/node pointers and re-validate them before execution, similarly to how it's done for UDFs.

@asfernandes
Copy link
Member

UDFs without their libraries, at least in recent versions, are allowed to be created even in the normal use (outside restore).

Should we do the same for UDRs?

@AlexPeshkoff
Copy link
Member

Do we check for parameters match when create UDR?

@asfernandes
Copy link
Member

Do we check for parameters match when create UDR?

Please elaborate.

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Apr 11, 2022

With UDF we have absolutely no ways to check for parameters match (and this is what a lot related vulnerabilities are using).
With UDR we can ensure that format of messages does match what UDR expects before calling it. I've never reviewed how and where are that checks organized but it's more or less obvious that in some cases error to be raised: for example when one tries to cast floating point to boolean or number of fields in messages do not match.

If some of that checks are performed at create time then adding UDR without the library seems to be bad idea, otherwise - why not.

@asfernandes
Copy link
Member

UDR may accept metadata defined structure in its message or change it (its message).

At this time, if UDR does not reject (throwing), incompatible message conversion may be created. Error will happen in runtime. We can make this better.

Now when creating UDR (via DDL or restore), it loads metadata and error about undefined library happens.

If we make it relaxed as UDF it may be worse as user error will be deferred to execution.

But as external engines is layered it seems difficult to at DDL creation only accept error about undefined library.

So it looks like to better just allow every type of error in creation via restore (not loading UDR metadata) and maintain DDL metadata loading which may detect error sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment