outerbase / studio

A lightweight SQLite graphical client on your browser. It also support connecting to LibSQL/Turso/rqlite
https://libsqlstudio.com
GNU Affero General Public License v3.0
336 stars 20 forks source link

fix: handle values according to their received type, not the column-defined type (fixes #137) #140

Closed rentalhost closed 1 month ago

rentalhost commented 1 month ago

Since SQLite is flexible with types, we can't blindly trust the column-defined type. Instead, we now determine the display type based on the type of the received data.

Here's how it works:

Additional improvements:

Additional notes:

vercel[bot] commented 1 month ago

@rentalhost is attempting to deploy a commit to the Mediaload Team on Vercel.

A member of the Team first needs to authorize it.

invisal commented 1 month ago

This approach has several short-coming

I believe you are in the right track, just need a few twist:

if (
  header.dataType === TableColumnDataType.INTEGER &&
  databaseDriver.supportBigInt() &&
  isBigNumber(value)
) {
  // BigNumberCell
} else if ((
  header.dataType === TableColumnDataType.INTEGER ||
  header.dataType === TableColumnDataType.REAL
) && isNumber(value)) {
  // NumberCell
} else if (
  header.dataType === TableColumnDataType.TEXT &&
  isString(value)
) {
  // TextCell
} else {
  // GenericCell
}

function isBigNumber(value) {
  if (typeof value === 'bigint') return true;
  if (value === null) return true;
  if (value === undefined) return true;

  try {
    BigInt(value);
    return true;
  } catch {
    return false;
  }
}

function isNumber(value) {
  if (typeof value === 'number') return true;
  if (value === null) return true;
  if (value === undefined) return true;
  return Number.isFinite(value);
}

function isString(value) {
  if (typeof value === 'string') return true;
  if (value === null) return true;
  if (value === undefined) return true;
  return false;
}
rentalhost commented 1 month ago

Perfect!

I tried to compare as much as I could between the current version and the proposed one, always aiming to keep the current behavior intact while only modifying the issue with NULL where it shouldn’t appear. So here are a few points:


Essa tradução mantém a clareza e o contexto técnico do original.

rentalhost commented 1 month ago

Regarding the last point, just to be sure I understood correctly: you were making a statement, not asking a question, right? If it’s a question, SQLite will automatically convert a value that looks like a number to an INTEGER when it's received in an INTEGER field, rather than keeping it as the wrong type, TEXT, if that’s the case.

invisal commented 1 month ago

Regarding the last point, just to be sure I understood correctly: you were making a statement, not asking a question, right? If it’s a question, SQLite will automatically convert a value that looks like a number to an INTEGER when it's received in an INTEGER field, rather than keeping it as the wrong type, TEXT, if that’s the case.

I was not sure what was the sqlite behavior, so I was raising concern.

@rentalhost lets me test further. It looks good so far.

rentalhost commented 1 month ago

This is specified in the Type Affinity section, specifically in section 3.4.

Note that in the first INSERT example, even though all VALUES are sent as strings, they are converted to their affinity types according to the column type. So, "500.0" is kept as a string in a TEXT column but is converted to an integer for the INTEGER column.

In the third example, the test is done using 500 as an integer literal, so it is cast to a string in the TEXT column but remains an integer in the INTEGER column.