FirebirdSQL / firebird

Firebird server, client and tools
https://www.firebirdsql.org/
1.19k stars 204 forks source link

Extract external routines in one pass #8054

Open vasiliy-yashkov opened 2 months ago

vasiliy-yashkov commented 2 months ago

Extract external procedures and functions in one pass because they have no BLR and cannot have dependencies.

asfernandes commented 2 months ago

I think this is going to cause problems.

When external routines are created, engine immediately calls external code, that can interact with existing metadata.

dyemanov commented 2 months ago

I think this is going to cause problems.

When external routines are created, engine immediately calls external code, that can interact with existing metadata.

Hmmm. Maybe theoretically this is possible, but is it really our problem? When UDR is created we have no idea about the logic inside the external module. How can we be expected to pre-create some metadata in advance? What if the external code would depend on some pre-existing data too?

asfernandes commented 2 months ago

I think this is going to cause problems. When external routines are created, engine immediately calls external code, that can interact with existing metadata.

Hmmm. Maybe theoretically this is possible, but is it really our problem? When UDR is created we have no idea about the logic inside the external module. How can we be expected to pre-create some metadata in advance? What if the external code would depend on some pre-existing data too?

Nothing different than PSQL routines. We shouldn't care about routines content, but we do to make their actual usage work as we don't know their creation/update order.

For example, it may receive another procedure name in its body, inspect it and precompute things for fast transformation of its result to json in call time.

The thing is that external routines may do things at "compilation" phase, and this things may be interaction with metadata.

Note also that UDR is a layer on top of external routines. External routines may do more things and it still is a public API.

dyemanov commented 1 month ago

I surely missed the "compilation stage" external calls, could you please point me where it happens?

asfernandes commented 1 month ago

I surely missed the "compilation stage" external calls, could you please point me where it happens?

If you put breakpoint in UdrEngine.cpp at

IExternalFunction* Engine::makeFunction(ThrowStatusWrapper* status, IExternalContext* context,
    IRoutineMetadata* metadata, IMetadataBuilder* inBuilder, IMetadataBuilder* outBuilder)
{
    return FB_NEW SharedFunction(status, this, context, metadata, inBuilder, outBuilder);
}

It will be called from DFW.

This also prevents problematic routines to be created:

create function div (
    n1 integer,
    n2 integer
) returns double precision
    external name 'udf_compat!ERROR'
    engine udr;

I may accept a change to only load/validate routines when they are called for the first time. But that should also happen when call is present in another PSQL routine, i.e., PSQL_FUNC is created and calls EXT_FUNC, then EXT_FUN should be loaded only when it's called.

It may have the advantage of declare something that is not yet present/configured in the machine. At same time, it will defer erroneous declaration.

But this patch alone is problematic IMO.

dyemanov commented 1 month ago

Thanks. I asked because there may be a difference. PSQL routines are compiled into BLR during DDL execution -- this is the first stage when dependent objects should already exist (METD should be able to find them). Then at DFW time dependencies are tracked (BLR is parsed) and this is the second stage. For external routines we have only the second stage (DFW time) dependencies access. At this time all dependencies already exist in the database. They're not committed yet, but visible for the current transaction. So I'm wondering whether it's enough to satisfy the possible external calls, given that ISQL extracts all metadata to be executed in one transaction.