Thread: Segmentation fault in volatile c function

Hi,

I'm creating my first ever extension. The function that I'm trying to write takes the schema and name of a table, and
addsit in a table with all the tables the extension follows. 
In that table I want the schema, name and oid of the table.

I created a C function for that. Here is the code

```
Datum add_mygreat_table(PG_FUNCTION_ARGS);

PG_FUNCTION_INFO_V1(add_mygreat_table);

Datum add_mygreat_table(PG_FUNCTION_ARGS) {
  char *tableSchema,
    *tableName;

  Oid tableNamespace,
    relationId;

  tableSchema = PG_GETARG_CSTRING(0);
  tableName = PG_GETARG_CSTRING(1);

  tableNamespace = get_namespace_oid(tableSchema, false);
  relationId = get_relname_relid(tableName, tableNamespace);

  if (relationId == NULL)
  {
    ereport(ERROR, (errmsg("could not create remote table: "
                           "relation does not exist")));
  }

  Datum values[3];
  bool isNulls[3];
  memset(isNulls, false, sizeof(isNulls));

  values[0] = CStringGetTextDatum(tableSchema);
  values[1] = CStringGetTextDatum(tableName);
  values[2] = ObjectIdGetDatum(relationId);

  Relation greatTable = table_open(MyGreatTablesRelationId(), RowExclusiveLock);

  HeapTuple tuple = heap_form_tuple(RelationGetDescr(greatTable), values, isNulls);

  /*
   * CatalogTupleInsert() is originally for PostgreSQL's catalog. However,
   * it is used at here for convenience.
   */
  CatalogTupleInsert(greatTable, tuple);

  table_close(greatTable, RowExclusiveLock);
  heap_freetuple(tuple);

  CommandCounterIncrement();

  ereport(NOTICE,
          (errmsg("Table %s is now a great table", tableName)));

  PG_RETURN_BOOL(true);
}
```


And in the sql I have

```
CREATE OR REPLACE FUNCTION mygreat.add_mygreat_table(cstring, cstring)
  RETURNS bool
  LANGUAGE C STRICT IMMUTABLE
AS '$libdir/mygreat',
$$add_mygreat_table$$;
```


Here is my problem, it works as expected when the table doesn't exist.

```
mygreat_test=# create extension hydra;

CREATE EXTENSION
mygreat_test=# select mygreat.add_mygreat_table('public', 'sampledata3');
ERROR:  could not create remote table: relation does not exist
```

However if the table exists it just completely crashes

```
hydra_test=# select hydra.add_remote_table('snowflake', 'public', 'sampledata2');
NOTICE:  Table sampledata2 is now a mygreat table
server closed the connection unexpectedly
    This probably means the server terminated abnormally
    before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
```

I figured that it was because of the `IMMUTABLE` as the function does modify the database. And according to
documentation

```
IMMUTABLE indicates that the function cannot modify the database and always returns the same result when given the same
argumentvalues; t 
```

So I tried creating it as a `VOLATILE` function.


```
CREATE OR REPLACE FUNCTION hydra.add_remote_table(cstring, cstring)
  RETURNS bool
  LANGUAGE C STRICT VOLATILE
AS '$libdir/mygreat',
$$add_mygreat_table$$;
CREATE FUNCTION
```

But now it just always crashes, and what used to return the proper error message when ran against a table that doesn't
existnow returns 

```
mygreat_test=# select mygreat.add_mygreat_table('public', 'sampledata3');
server closed the connection unexpectedly
    This probably means the server terminated abnormally
    before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
```

In the logs, for the latest, I get

```
2021-10-27 12:28:02.712 PDT [44115] LOG:  statement: select mygreat.add_mygreat_table('public', 'sampledata3');
2021-10-27 12:28:02.718 PDT [43936] LOG:  server process (PID 44115) was terminated by signal 11: Segmentation fault:
11
2021-10-27 12:28:02.718 PDT [43936] DETAIL:  Failed process was running: select mygreat.add_mygreat_table('public',
'sampledata3');
2021-10-27 12:28:02.719 PDT [43936] LOG:  terminating any other active server processes
```

What am I doing wrong? Can anyone help me with this?

Thank you!

On Wed, Oct 27, 2021 at 12:39 PM Louise Grandjonc
<louve.grandjonc@gmail.com> wrote:
> I'm creating my first ever extension. The function that I'm trying to write takes the schema and name of a table, and
addsit in a table with all the tables the extension follows. 
> In that table I want the schema, name and oid of the table.
>
> I created a C function for that. Here is the code

I don't think that using cstring as an argument is appropriate here.
That's only used for "internal" functions that are called through C,
rather than being called through SQL.

It would probably be better to use a regclass argument instead. Take a
look at the example of (say) amcheck's bt_index_check() or
verify_heapam() functions to see how that's done. Not all C functions
use this built-in way of specifying a particular relation from SQL (a
few older ones really do just pass a string some other way), but it's
definitely considered the way to go.

Note that you can do stuff like this from SQL:

pg@regression:5432 [2999218]=# select 'pg_class'::regclass::oid;
  oid
───────
 1,259
(1 row)

--
Peter Geoghegan

Thank you! I used that. The segmentation fault came from a later code in my hook. But that helped.

> On Oct 27, 2021, at 12:47 PM, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Wed, Oct 27, 2021 at 12:39 PM Louise Grandjonc
> <louve.grandjonc@gmail.com> wrote:
>> I'm creating my first ever extension. The function that I'm trying to write takes the schema and name of a table,
andadds it in a table with all the tables the extension follows. 
>> In that table I want the schema, name and oid of the table.
>>
>> I created a C function for that. Here is the code
>
> I don't think that using cstring as an argument is appropriate here.
> That's only used for "internal" functions that are called through C,
> rather than being called through SQL.
>
> It would probably be better to use a regclass argument instead. Take a
> look at the example of (say) amcheck's bt_index_check() or
> verify_heapam() functions to see how that's done. Not all C functions
> use this built-in way of specifying a particular relation from SQL (a
> few older ones really do just pass a string some other way), but it's
> definitely considered the way to go.
>
> Note that you can do stuff like this from SQL:
>
> pg@regression:5432 [2999218]=# select 'pg_class'::regclass::oid;
>  oid
> ───────
> 1,259
> (1 row)
>
> --
> Peter Geoghegan