FWD » Core Development » Database

We have had many users stumble with the installation of the native UDFs. In the event the directory does not specify to use Java UDFs, we should, upon startup of the FWD server, automatically detect whether the native UDFs are installed in the persistent databases for which the current FWD server is authoritative (and of course, for which native UDFs are supported). If they are not already installed, the native UDFs should be installed/configured.

There is a problem with "on the fly" creation of native UDFs.
Many of them a declared as "leakproof" but such UDFs can only be defined by an admin while the application normally starts with a 'normal' user credentials.
Actually "leakproof" UDF attribute is not absolutely necessary. Here is an excerpt from the PG documentation:

LEAKPROOF indicates that the function has no side effects. It reveals no information about its arguments other than by its return value. For example, a function which throws an error message for some argument values but not others, or which includes the argument values in any error message, is not leakproof. This affects how the system executes queries against views created with the security_barrier option or tables with row level security enabled. The system will enforce conditions from security policies and security barrier views before any user-supplied conditions from the query itself that contain non-leakproof functions, in order to prevent the inadvertent exposure of data. Functions and operators marked as leakproof are assumed to be trustworthy, and may be executed before conditions from security policies and security barrier views. In addition, functions which do not take arguments or which are not passed any arguments from the security barrier view or table do not have to be marked as leakproof to be executed before security conditions. See CREATE VIEW and Section 41.5. This option can only be set by the superuser.

Should I remove "LEAKPROOF" attribute from the UDFs definitions?
Thank you.

Another problem is with creation of the udf schema. The create schema if not exists udf statament also can be executed only by admin.
Any suggestions?
Thank you.

Igor Skornyakov wrote:

There is a problem with "on the fly" creation of native UDFs.
Many of them a declared as "leakproof" but such UDFs can only be defined by an admin while the application normally starts with a 'normal' user credentials.
Actually "leakproof" UDF attribute is not absolutely necessary. Here is an excerpt from the PG documentation:
[...]

Should I remove "LEAKPROOF" attribute from the UDFs definitions?
Thank you.

It seems many of the UDFs should be leakproof by design, but others would definitely raise an error for certain arguments and not others. I have not carefully reviewed the UDFs with this constraint in mind. I think it is useful to retain this attribute where it is in fact true.

Igor Skornyakov wrote:

Another problem is with creation of the udf schema. The create schema if not exists udf statament also can be executed only by admin.
Any suggestions?
Thank you.

This makes sense. We could add an admin_username and admin_password in the .../database/<database_name>/orm/connection/ section of the directory, with the assumption that directory.xml is a secured document (in fact, we already have this assumption w.r.t. other information in the directory, including the non-superuser database role's username and password). The checking/installing of UDFs at server startup would use a dedicated connection; we would not set up a connection pool for admin connections. Eventually, we'll need to implement secure, passwordless authentication of a connection over JDBC, but not necessarily in the scope of this task.

This makes sense. We could add an admin_username and admin_password in the .../database/<database_name>/orm/connection/ section of the directory, with the assumption that directory.xml is a secured document (in fact, we already have this assumption w.r.t. other information in the directory, including the non-superuser database role's username and password). The checking/installing of UDFs at server startup would use a dedicated connection; we would not set up a connection pool for admin connections. Eventually, we'll need to implement secure, passwordless authentication of a connection over JDBC, but not necessarily in the scope of this task.

Got it. Thank you.

Code review missing-udf.diff:

It seems this should work.

Please note we will have to support this feature for every dialect which supports native UDFs. While I prefer not to have the technical debt of PostgreSQL-specific logic in ScriptRunner, I realize the proper division of common code vs. dialect-specific code may not be obvious without the benefit of looking at one or two other dialect implementations. For now, it is OK to leave PostgreSQL-specific code in ScriptRunner, but in the places where it looks likely that some refactoring will be necessary when native UDF support is added for other dialects, please put in more comments (including some TODO markers). In particular, please comment the COUNT instance variable, as this surely will need to differ between dialects.

Eric Faulhaber wrote:

Code review missing-udf.diff:

It seems this should work.

Please note we will have to support this feature for every dialect which supports native UDFs. While I prefer not to have the technical debt of PostgreSQL-specific logic in ScriptRunner, I realize the proper division of common code vs. dialect-specific code may not be obvious without the benefit of looking at one or two other dialect implementations. For now, it is OK to leave PostgreSQL-specific code in ScriptRunner, but in the places where it looks likely that some refactoring will be necessary when native UDF support is added for other dialects, please put in more comments (including some TODO markers). In particular, please comment the COUNT instance variable, as this surely will need to differ between dialects.

I was thinking about refactoring to extract dialect-specific logic to the Dialect class and its childs.
However, finally I came to the same conclusion you're mentioned - it is difficult to decide how it should look like for the dialects other than PostgreSQL and a premature abstraction can be wrong and even make adding support for the new dialects more complicated.

BTW: I think that right way to re-work ScriptRunner for more dialects is to create its dialect-specific versions instead of moving dialect-specific code to the corresponding Dialect. The common parts will remain in the base class. This approach seems to be more flexible and maintanable.