pgrx icon indicating copy to clipboard operation
pgrx copied to clipboard

Allow expressions for extension_sql and extension_sql_file macros

Open JamesGuthrie opened this issue 3 years ago • 8 comments
trafficstars

This builds on the open PR: https://github.com/zombodb/pgx/pull/303, allowing for arbitrary expressions in the extension_sql_file macro.

JamesGuthrie avatar Jan 27 '22 09:01 JamesGuthrie

I'd like to note that one of the main caveats with the change to extension_sql_file is that it requires setting the name attribute. This stems from the fact that we can't evaluate the expression to determine the name.

It's also possibly not intuitive that it only works for a certain subset of expressions and macros, namely those which are fully resolved at compile time.

JamesGuthrie avatar Jan 27 '22 09:01 JamesGuthrie

I'm wondering if there is a way we can preserve the auto-naming if the user passes a syn::LitStr instead of an syn::Expr...

I was wondering the same. I'm not familiar enough with proc macros or syn to know if that is possible.

JamesGuthrie avatar Jan 27 '22 17:01 JamesGuthrie

This PR is causing some strangeness on pgx-tests's sql generation.

I attempted to add the following to pgx-tests/src/tests/extension_sql_tests.rs:

extension_sql_file!(
    "../../sql/example.sql",
    name = "example_file",
);

Where ../../sql/example.sql contained CREATE TABLE example();.

Once I did this all extension_sql and extension_sql macros inside only that module disappeared from the linkage in sql-generator:

ana@autonoma:~/git/zombodb/pgx/pgx-tests$ nm -D ../target/debug/libpgx_tests.so | grep __pgx_internals_sql
00000000000f3940 T __pgx_internals_sql_create_complex_shell_type
00000000000f5390 T __pgx_internals_sql_create_complex_type
00000000000aab80 T __pgx_internals_sql_example_file
00000000000aa8d0 T __pgx_internals_sql_extension_sql_dynamic
00000000000aa6e0 T __pgx_internals_sql_extension_sql_table
ana@autonoma:~/git/zombodb/pgx/pgx-tests$ nm -D ../target/debug/sql-generator | grep __pgx_internals_sql
00000000001a04c0 T __pgx_internals_sql_create_complex_shell_type
00000000001a16b0 T __pgx_internals_sql_create_complex_type

On develop this works:

ana@autonoma:~/git/zombodb/pgx/pgx-tests$ nm -D ../target/debug/sql-generator | grep __pgx_internals_sql
00000000001a1940 T __pgx_internals_sql_create_complex_shell_type
00000000001a2ad0 T __pgx_internals_sql_create_complex_type
0000000000166cc0 T __pgx_internals_sql_example_file
0000000000166b30 T __pgx_internals_sql_extension_sql_dynamic
00000000001669a0 T __pgx_internals_sql_extension_sql_table


ana@autonoma:~/git/zombodb/pgx/pgx-tests$ nm -D ../target/debug/libpgx_tests.so | grep __pgx_internals_sql
00000000000f3390 T __pgx_internals_sql_create_complex_shell_type
00000000000f4d80 T __pgx_internals_sql_create_complex_type
0000000000120ff0 T __pgx_internals_sql_example_file
0000000000120e60 T __pgx_internals_sql_extension_sql_dynamic
0000000000120cd0 T __pgx_internals_sql_extension_sql_table

If I also change:

extension_sql!(
    "CREATE TABLE extension_sql_dynamic();\n",
    name = "extension_sql_dynamic"
);

The code starts working again:

ana@autonoma:~/git/zombodb/pgx/pgx-tests$ nm -D ../target/debug/libpgx_tests.so | grep __pgx_internals_sql
0000000000189300 T __pgx_internals_sql_create_complex_shell_type
000000000018ad50 T __pgx_internals_sql_create_complex_type
000000000016dd80 T __pgx_internals_sql_example_file
000000000016db90 T __pgx_internals_sql_extension_sql_dynamic
000000000016d9a0 T __pgx_internals_sql_extension_sql_table
ana@autonoma:~/git/zombodb/pgx/pgx-tests$ nm -D ../target/debug/sql-generator | grep __pgx_internals_sql
0000000000132c10 T __pgx_internals_sql_create_complex_shell_type
0000000000133e00 T __pgx_internals_sql_create_complex_type
000000000018bee0 T __pgx_internals_sql_example_file
000000000018bcf0 T __pgx_internals_sql_extension_sql_dynamic
000000000018bb00 T __pgx_internals_sql_extension_sql_table

Hoverbear avatar Jan 28 '22 20:01 Hoverbear

So this appears to be because of the format!() macro usage, as I can use concat!() just fine. This is something I'd prefer to resolve before merge...

Hoverbear avatar Jan 31 '22 17:01 Hoverbear

I think we're going to hold this until 0.3.1 until we can wrangle this. We'd like to cut a 0.3.0 soon.

Hoverbear avatar Feb 01 '22 18:02 Hoverbear

I don't really understand why it only happens in pgx-tests...

Hoverbear avatar Feb 04 '22 17:02 Hoverbear

In #441 we significantly refactored some of this code.... There's good news and bad news:

  • It will not be a very nice merge -- it may be easier to look at the diff and redo the changes. (edit pgx_utils/src/sql_entity/graph only now)
  • It should eliminate those bugs I was detecting.

Hoverbear avatar Feb 25 '22 22:02 Hoverbear

@JamesGuthrie is this still of interest to you?

Hoverbear avatar Jun 01 '22 16:06 Hoverbear

Closing this as stale, feel free to reopen this or open a new PR.

workingjubilee avatar Nov 02 '22 23:11 workingjubilee