Problem with BYTEAOID type When Iterate Foreign scan

hankh92 commented 3 years ago

When select foreign table with bytea type, it ran to crash bug for example:

+------------+----+
| v          | id |
+------------+----+
| 0xAA       |  1 |
| 0xAA       |  2 |
+------------+----+
2 rows in set (0.09 sec)

postgres=# select *  from bytea_test_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.
!?>

The problem is cause by the code block

        /*
         * Release locally palloc'd space.  and values of pass-by-reference
         * datums, as well. For BYTEAOID case does not free
         */
        for (i = 0; i < natts; i++)
        {
            if (dvalues[i] && !TupleDescAttr(attinmeta->tupdesc, i)->attbyval)
                pfree(DatumGetPointer(dvalues[i]));
        }
        pfree(dvalues);
        pfree(nulls);

Which cause the crash when fetch the 2nd row

libc.so.6!__memcpy_ssse3_back() (\usr\src\debug\glibc-2.17-c758a686\sysdeps\x86_64\multiarch\memcpy-ssse3-back.S:2254)
libmysqlclient.so!memcpy(const void * restrict __src, void * restrict __dest) (\usr\include\bits\string3.h:51)
libmysqlclient.so!fetch_result_bin(MYSQL_BIND * param, MYSQL_FIELD * field, uchar ** row) (\var\lib\pb2\sb_1-410081-1600867876.83\rpm\BUILD\mysql-8.0.22\mysql-8.0.22\libmysql\libmysql.cc:3633)
libmysqlclient.so!stmt_fetch_row(uchar * row, MYSQL_STMT * stmt) (\var\lib\pb2\sb_1-410081-1600867876.83\rpm\BUILD\mysql-8.0.22\mysql-8.0.22\libmysql\libmysql.cc:4009)
libmysqlclient.so!mysql_stmt_fetch(MYSQL_STMT * stmt) (\var\lib\pb2\sb_1-410081-1600867876.83\rpm\BUILD\mysql-8.0.22\mysql-8.0.22\libmysql\libmysql.cc:4053)
mysql_fdw.so!mysqlIterateForeignScan(ForeignScanState * node) (\home\tsdv\postgres\PGSpider\contrib\mysql_fdw\mysql_fdw.c:834)
ForeignNext(ForeignScanState * node) (\home\tsdv\postgres\PGSpider\src\backend\executor\nodeForeignscan.c:54)
ExecScanFetch(ScanState * node, ExecScanAccessMtd accessMtd, ExecScanRecheckMtd recheckMtd) (\home\tsdv\postgres\PGSpider\src\backend\executor\execScan.c:133)
ExecScan(ScanState * node, ExecScanAccessMtd accessMtd, ExecScanRecheckMtd recheckMtd) (\home\tsdv\postgres\PGSpider\src\backend\executor\execScan.c:182)
ExecForeignScan(PlanState * pstate) (\home\tsdv\postgres\PGSpider\src\backend\executor\nodeForeignscan.c:115)
ExecProcNode(PlanState * node) (\home\tsdv\postgres\PGSpider\src\include\executor\executor.h:245)
ExecutePlan(EState * estate, PlanState * planstate, _Bool use_parallel_mode, CmdType operation, _Bool sendTuples, uint64 numberTuples, ScanDirection direction, DestReceiver * dest, _Bool execute_once) (\home\tsdv\postgres\PGSpider\src\backend\executor\execMain.c:1658)
standard_ExecutorRun(QueryDesc * queryDesc, ScanDirection direction, uint64 count, _Bool execute_once) (\home\tsdv\postgres\PGSpider\src\backend\executor\execMain.c:376)
ExecutorRun(QueryDesc * queryDesc, ScanDirection direction, uint64 count, _Bool execute_once) (\home\tsdv\postgres\PGSpider\src\backend\executor\execMain.c:320)
PortalRunSelect(Portal portal, _Bool forward, long count, DestReceiver * dest) (\home\tsdv\postgres\PGSpider\src\backend\tcop\pquery.c:912)
PortalRun(Portal portal, long count, _Bool isTopLevel, _Bool run_once, DestReceiver * dest, DestReceiver * altdest, QueryCompletion * qc) (\home\tsdv\postgres\PGSpider\src\backend\tcop\pquery.c:756)
exec_simple_query(const char * query_string) (\home\tsdv\postgres\PGSpider\src\backend\tcop\postgres.c:1300)
PostgresMain(int argc, char ** argv, const char * dbname, const char * username) (\home\tsdv\postgres\PGSpider\src\backend\tcop\postgres.c:4588)
BackendRun(Port * port) (\home\tsdv\postgres\PGSpider\src\backend\postmaster\postmaster.c:4536)
BackendStartup(Port * port) (\home\tsdv\postgres\PGSpider\src\backend\postmaster\postmaster.c:4220)

When pfree(DatumGetPointer(dvalues[i]) try to free the pointer value from dvalues[i] while the dvalues[i] in case of BYTEAOID is

        case BYTEAOID:
            SET_VARSIZE(column->value, column->length + VARHDRSZ);
            return PointerGetDatum(column->value);

Which mean it point to the buffer inside mysql_fetch() internal buffer, the the free routine will free the buffer used to fetch next row from mysql library.

Then I think in this case we should choose between:

Not free the buffer when pgtype is BYTEAOID.
Dont need to free the buffer because all allocation is inside memory context and it can automatically release.
Use allocation in mysql_convert_to_pg() to get the buffer of byteaOID, then we can free it normally as original source code.

surajkharage19 commented 3 years ago

Hi @hankh92,

Thank you for reporting this issue with analysis. I am able to reproduce the same. Your third approach seems good to me and I tried to implement the same in the attached v1 patch. It would be good if you could review that and share your feedback on the same.

Thanks and Regards, Suraj kharage FDW-378_v1.txt

surajkharage19 commented 3 years ago

Hi @hankh92,

We have fixed this issue under commit d83f62387c07c8546a5db98ac2c86aac64213c14, can you please verify the same by pulling the latest sources?

Thanks for reporting this and suggestions.

hankh92 commented 3 years ago

Hi, @surajkharage19 surajkharage19 Thanks for your fixing. I have reviewed and it is verified.

EnterpriseDB / mysql_fdw

Problem with BYTEAOID type When Iterate Foreign scan #221