siara-cc / esp32_arduino_sqlite3_lib

Sqlite3 Arduino library for ESP32
Apache License 2.0
350 stars 66 forks source link

solution to SQL error: disk I/O / SQLITE_IOERR (10): temp file support #81

Open winkelict opened 7 months ago

winkelict commented 7 months ago

in esp32.cpp the comment for this function specifies this:

static int ESP32Open(
  sqlite3_vfs *pVfs,              /* VFS */
  const char *zName,              /* File to open, or 0 for a temp file */

however, this is not implemented in esp32:

 if( zName==0 ){
    return SQLITE_IOERR;
  }

In some issues this was already mentioned, but i did not find a complete implementation/fix. https://github.com/siara-cc/esp32_arduino_sqlite3_lib/issues/55 https://github.com/siara-cc/esp32_arduino_sqlite3_lib/issues/41

ive made a drop in replacement for esp32open that will generate a temporary file name and open that in w+ mode. also, to overcome the problem that different filesystems have differents roots (like /sd) ive added parsing out the root dir when a .db file is opened. This way the temp files will always be in the same folder as the .db file. Also, the temp file is unlinked right away so does not have to be cleaned up / deleted on file closing as the sqlite library specifies. This is the function:

static char dbrootpath[MAXPATHNAME+1];

/*
** Open a file handle.
*/
static int ESP32Open(
  sqlite3_vfs *pVfs,              /* VFS */
  const char *zName,              /* File to open, or 0 for a temp file */
  sqlite3_file *pFile,            /* Pointer to ESP32File struct to populate */
  int flags,                      /* Input SQLITE_OPEN_XXX flags */
  int *pOutFlags                  /* Output SQLITE_OPEN_XXX flags (or NULL) */
){
  static const sqlite3_io_methods ESP32io = {
    1,                            /* iVersion */
    ESP32Close,                    /* xClose */
    ESP32Read,                     /* xRead */
    ESP32Write,                    /* xWrite */
    ESP32Truncate,                 /* xTruncate */
    ESP32Sync,                     /* xSync */
    ESP32FileSize,                 /* xFileSize */
    ESP32Lock,                     /* xLock */
    ESP32Unlock,                   /* xUnlock */
    ESP32CheckReservedLock,        /* xCheckReservedLock */
    ESP32FileControl,              /* xFileControl */
    ESP32SectorSize,               /* xSectorSize */
    ESP32DeviceCharacteristics     /* xDeviceCharacteristics */
  };

  ESP32File *p = (ESP32File*)pFile; /* Populate this structure */
  int oflags = 0;                 /* flags to pass to open() call */
  char *aBuf = 0;
    char mode[5];
      //Serial.println("fn: Open");

    strcpy(mode, "r");

  if( flags&SQLITE_OPEN_MAIN_JOURNAL ){
    aBuf = (char *)sqlite3_malloc(SQLITE_ESP32VFS_BUFFERSZ);
    if( !aBuf ){
      return SQLITE_NOMEM;
    }
  }

    if( flags&SQLITE_OPEN_CREATE || flags&SQLITE_OPEN_READWRITE 
          || flags&SQLITE_OPEN_MAIN_JOURNAL ) {
    struct stat st;
    memset(&st, 0, sizeof(struct stat));
    int rc = (zName == 0 ? -1 : stat( zName, &st ));
    //Serial.println(zName);
        if (rc < 0) {
      strcpy(mode, "w+");
      //int fd = open(zName, (O_CREAT | O_RDWR | O_EXCL), S_IRUSR | S_IWUSR);
      //close(fd);
      //oflags |= (O_CREAT | O_RDWR);
      //Serial.println("Create mode");
    } else
      strcpy(mode, "r+");
    }

  memset(p, 0, sizeof(ESP32File));

  //p->fd = open(zName, oflags, 0600);
  //p->fd = open(zName, oflags, S_IRUSR | S_IWUSR);
  if (zName == 0) {
    //generate a temporary file name
    char *tName = tmpnam(NULL);
    tName[4] = '_';
    size_t len = strlen(dbrootpath);
    memmove(tName + len, tName, strlen(tName) + 1);
    memcpy(tName, dbrootpath, len);    
    p->fp = fopen(tName, mode);
    //https://stackoverflow.com/questions/64424287/how-to-delete-a-file-in-c-using-a-file-descriptor
    //for temp file, then no need to handle in esp32close
    unlink(tName);
    //Serial.println("Temporary file name generated: " + String(tName) + " mode: " + String(mode));
  }
  else {
    //detect database root as folder for temporary files, every newly openened db will change this path
    //this mainly fixes that vfs's have their own root name like /sd
    char *ext = strrchr(zName, '.');
    bool isdb = false;
    if (ext) {
      isdb = (strcmp(ext+1,"db") == 0);
    }
    if (isdb) {      
      char zDir[MAXPATHNAME+1];
      int i=0;
      strcpy(zDir,zName);

      for(i=1; zDir[i]!='/'; i++) {};
      zDir[i] = '\0';

      strcpy(dbrootpath, zDir);
    }

    p->fp = fopen(zName, mode);
  }

  if( p->fp<=0){
    if (aBuf)
      sqlite3_free(aBuf);
    //Serial.println("Can't open");
    return SQLITE_CANTOPEN;
  }
  p->aBuffer = aBuf;

  if( pOutFlags ){
    *pOutFlags = flags;
  }
  p->base.pMethods = &ESP32io;
  //Serial.println("fn:Open:Success");
  return SQLITE_OK;
}

Or should i open a pull request?

winkelict commented 7 months ago

there are quite some reasons sqlite uses temp files:

https://www.sqlite.org/tempfiles.html

SQLite currently uses nine distinct types of temporary files:

Rollback journals Super-journals Write-ahead Log (WAL) files Shared-memory files Statement journals TEMP databases Materializations of views and subqueries Transient indices Transient databases used by VACUUM

in my case, add a sudden point, using a large query, without any database changes sqlite decided to make a temp file to speed up the query (i assume). It might have tried to make a transient index for this.

Because there are so many reasons sqlite can request a temp file it might be important to add the above function to esp32.cpp

savejeff commented 7 months ago

Hi, i've worked quite a bit on the sqlite file abstraction for my project. Do you by chance use PlatformIO? i might have a improved version of this lib. its pretty reliable

One problem with the current implementation in this repo is that the temp/journal file is in a 8K Buffer, not an actual file. i have change it so that i buffer often used sectors but handle the journal file as a regular file.

sqlite3 does not always directly writes or reads from the actual file if the query is to short. so a BEGIN TRANSACTION does not immediately create a journal file. only after some inserts it actually writes to it.

another problem is with the truncate operation that is needed by the sqlite3 lib when for example the journal file is truncated to 0 bytes after a COMMIT command or if VACUUM is used as the database file shrinks.

i have gotten all these running reliably

winkelict commented 7 months ago

hi, yes, i use platformio, i would be very interested to see your implementation!

winkelict commented 7 months ago

did you ever make a PR or send a zip to sierra-cc?

savejeff commented 7 months ago

did you ever make a PR or send a zip to sierra-cc?

i gave sierra a zip with adjusted but not directly ready to use code (it was already a little bit too far gone from the base code) but i don't know if he integrated it.

how should i send you the code? it will take me a little to prepare it so that i don't leak too much code

savejeff commented 7 months ago

here the repo with running code for ESP32, RP2040 and STM32. Teensy 4.1 should also work. it works with SD Card filesystems and Flash-based internal filesystem like SPIFFS and LittleFS

i recommend you extract the code you need

winkelict commented 6 months ago

thanks man! looks good, it evolved quite far from this original sqlite3 library right? will take me a while to figure out

savejeff commented 6 months ago

Yeah I have redonr basically the complete file interaction by switching to my personal jFS library that is a wrapper for each MCUs own SD library. But the general sqlite3 code is completely unchanged.

If you want you can exchange jFS with SdFat pretty easily as they basically have the same interface.

If you gave questions just open up an issue on the jSQlite repo

siara-cc commented 6 months ago

@savejeff Yes you had shared the changes that you had made for this issue. I started working on the info you have given but unfortunately have not been able to spend time to complete it. @winkelict Hi thanks for raising this if you have a PR I could test it and merge it

siara-cc commented 6 months ago

Hi @winkelict I have tested your dropin code suggestion and committed to this repo. Seems working. Thanks a lot!! @savejeff Thanks for your suggestions too!