Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
winkelict opened this issue Dec 8, 2023 · 10 comments
Open

Comments

@winkelict
Copy link

winkelict commented Dec 8, 2023

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.
#55
#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
Copy link
Author

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
Copy link

savejeff commented Dec 8, 2023

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
Copy link
Author

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

@winkelict
Copy link
Author

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

@savejeff
Copy link

savejeff commented Dec 8, 2023

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
Copy link

savejeff commented Dec 8, 2023

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
Copy link
Author

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

@savejeff
Copy link

savejeff commented Dec 9, 2023

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
Copy link
Owner

@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
Copy link
Owner

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants