Integer overflow in cstore_clean_table_resources()

[mertam]

Hello,

there is incorrect signed/unsigned integer conversion in cstore_clean_table_resources() function. Once the Oid goes over 2^31 -1, cstore files aren't deleted and become unreachable by postgres. In the other words, DROP FOREIGN TABLE statement won't free any data on disk. All of this happens in silence without any notice in logs - I understand why there is no logging of missing files, but it makes the issue even more serious since it starts happening in random point in time (from the perspective of user).

This issue caused me non-trivial problems on version 1.7.0. I hope for quick fix in main repository so others can be saved from hours of debugging.

This is the patch I'm currently using in my deployments:

diff --git a/cstore_fdw.c b/cstore_fdw.c
index 7bfac28..ed08a63 100644
--- a/cstore_fdw.c
+++ b/cstore_fdw.c
@@ -1356,8 +1356,8 @@ cstore_clean_table_resources(PG_FUNCTION_ARGS)
        struct stat fileStat;
        int statResult = -1;

-       appendStringInfo(filePath, "%s/%s/%d/%d", DataDir, CSTORE_FDW_NAME,
-                                        (int) MyDatabaseId, (int) relationId);
+       appendStringInfo(filePath, "%s/%s/%u/%u", DataDir, CSTORE_FDW_NAME,
+                                        MyDatabaseId, relationId);

        /*
         * Check to see if the file exist first. This is the only way to

Cheers, MM

[ljluestc]

#include <sys/stat.h>
#include <string.h>
#include "postgres.h"
#include "cstore_fdw.h"

PG_FUNCTION_INFO_V1(cstore_clean_table_resources);

void cstore_clean_table_resources(Oid relationId)
{
    StringInfoData filePath;
    struct stat fileStat;
    int statResult = -1;

    /* Initialize the file path string */
    initStringInfo(&filePath);

    /* Construct the file path using unsigned format specifiers for Oid values */
    appendStringInfo(filePath, "%s/%s/%u/%u", DataDir, CSTORE_FDW_NAME,
                     MyDatabaseId, relationId);

    /*
     * Check to see if the file exists first. This prevents errors when
     * attempting to delete non-existent files.
     */
    statResult = stat(filePath.data, &fileStat);
    if (statResult == 0)
    {
        /* File exists, delete it */
        if (unlink(filePath.data) != 0)
        {
            ereport(WARNING, (errmsg("could not delete file \"%s\": %m", filePath.data)));
        }
    }

    /* Clean up memory */
    pfree(filePath.data);
}