Hi, Alexander!
> A colleague of mine (cc'ed) reported that he was able to pass a NULL
> snapshot to index_beginscan() and it even worked to a certain degree.
>
> I took my toy extension [1] and replaced the argument with NULL as an
> experiment:
>
> ```
> eax=# CREATE EXTENSION experiment;
> CREATE EXTENSION
> eax=# SELECT phonebook_lookup_index('Alice');
> phonebook_lookup_index
> ------------------------
> -1
> (1 row)
>
> eax=# SELECT phonebook_insert('Bob', 456);
> phonebook_insert
> ------------------
> 1
> (1 row)
>
> eax=# SELECT phonebook_lookup_index('Alice');
> phonebook_lookup_index
> ------------------------
> -1
> (1 row)
>
> eax=# SELECT phonebook_insert('Alice', 123);
> phonebook_insert
> ------------------
> 2
> (1 row)
>
> eax=# SELECT phonebook_lookup_index('Alice');
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> ```
>
> So evidently it really works as long as the index doesn't find any
> matching rows.
>
> This could be really confusing for the extension authors so here is a
> patch that adds corresponding Asserts().
>
> [1]: https://github.com/afiskon/postgresql-extensions/tree/main/005-table-access
I think it's a nice catch and worth fixing. The one thing I don't
agree with is using asserts for handling the error that can appear
because most probably the server is built with assertions off and in
this case, there still will be a crash in this case. I'd do this with
report ERROR. Otherwise, the patch looks right and worth committing.
Kind regards,
Pavel Borisov.
