Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ export {
isDatabaseClient,
__table as applyDataTableOperations,
getTypeValidator,
inferSchema
inferSchema,
getSchema
} from "./table.js";
21 changes: 14 additions & 7 deletions src/table.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,18 +617,25 @@ export function coerceToType(value, type) {
}
}

export function getSchema(source) {
const {columns} = source;
let {schema} = source;
if (!isQueryResultSetSchema(schema)) {
schema = inferSchema(source, isQueryResultSetColumns(columns) ? columns : undefined);
return {schema, inferred: true};
}
return {schema, inferred: false};
}

// This function applies table cell operations to an in-memory table (array of
// objects); it should be equivalent to the corresponding SQL query. TODO Use
// DuckDBClient for data arrays, too, and then we wouldn’t need our own __table
// function to do table operations on in-memory data?
export function __table(source, operations) {
const input = source;
let {schema, columns} = source;
let inferredSchema = false;
if (!isQueryResultSetSchema(schema)) {
schema = inferSchema(source, isQueryResultSetColumns(columns) ? columns : undefined);
inferredSchema = true;
}
const schemaInfo = getSchema(source);
let {columns} = source;
let {schema, inferred} = schemaInfo;
Copy link
Member

@mbostock mbostock Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor simplification:

Suggested change
const schemaInfo = getSchema(source);
let {columns} = source;
let {schema, inferred} = schemaInfo;
let {columns} = source;
let {schema, inferred} = getSchema(source);

Also: I don’t think we should fix it now, but there’s a latent bug below where where we’re assuming that if source.columns is truthy, it’s a valid QueryResultSetColumns. That’s because this function is trying to preserve the same shape of output as the input; so, if the input had a source.columns, we return a result.columns. I don’t think we need to do that; we could just always return a result.schema instead, now that we’re inferring a schema as needed. But again, reasonable to do that as a separate change to minimize risk since it’s not directly related to this PR’s objective.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm yeah, now that .schema is always defined, do we even need to return .columns from this function or can we just remove it? or would that be a breaking change, since we export __table from stdlib?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I meant I think that __table can always return .schema (and never .columns). I don’t think that would be breaking… 🤷

// Combine column types from schema with user-selected types in operations
const types = new Map(schema.map(({name, type}) => [name, type]));
if (operations.types) {
Expand All @@ -640,7 +647,7 @@ export function __table(source, operations) {
if (colIndex > -1) schema[colIndex] = {...schema[colIndex], type};
}
source = source.map(d => coerceRow(d, types, schema));
} else if (inferredSchema) {
} else if (inferred) {
// Coerce data according to new schema, unless that happened due to
// operations.types, above.
source = source.map(d => coerceRow(d, types, schema));
Expand Down
75 changes: 71 additions & 4 deletions test/table-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import {
getTypeValidator,
inferSchema,
makeQueryTemplate,
__table
__table,
getSchema
} from "../src/table.js";
import assert from "assert";

Expand Down Expand Up @@ -805,13 +806,37 @@ describe("__table", () => {
"b",
"c"
]);
source.schema = [
{name: "a", type: "integer", inferred: "integer"},
assert.deepStrictEqual(__table(source, operations).schema, [
{name: "nameA", type: "integer", inferred: "integer"},
{name: "b", type: "integer", inferred: "integer"},
{name: "c", type: "integer", inferred: "integer"}
]);
});

it("__table type assertions", () => {
const operations = {
...EMPTY_TABLE_DATA.operations,
types: [{name: "a", type: "string"}]
};
const expected = [
{a: "1", b: 2, c: 3},
{a: "2", b: 4, c: 6},
{a: "3", b: 6, c: 9}
];
expected.schema = [
{name: "a", type: "string", inferred: "integer"},
{name: "b", type: "integer", inferred: "integer"},
{name: "c", type: "integer", inferred: "integer"}
];
assert.deepStrictEqual(__table(source, operations), expected);
source.columns = ["a", "b", "c"];
assert.deepStrictEqual(__table(source, operations).columns, [
"a",
"b",
"c"
]);
assert.deepStrictEqual(__table(source, operations).schema, [
{name: "nameA", type: "integer", inferred: "integer"},
{name: "a", type: "string", inferred: "integer"},
{name: "b", type: "integer", inferred: "integer"},
{name: "c", type: "integer", inferred: "integer"}
]);
Expand Down Expand Up @@ -1213,3 +1238,45 @@ describe("coerceToType", () => {
// Note: if type is "raw", coerceToType() will not be called. Instead, values
// will be returned from coerceRow().
});

describe("getSchema", () => {
let source;

beforeEach(() => {
source = [
{a: 1, b: "foo"},
{a: 2, b: "bar"}
];
source.schema = [
{name: "a", type: "integer", inferred: "integer"},
{name: "b", type: "string", inferred: "string"}
];
});


it("respects schema from source, if one exists", () => {
const {schema, shouldCoerce} = getSchema(source);
assert.strictEqual(shouldCoerce, false);
assert.strictEqual(schema, source.schema);
});

it("infers schema if source has no schema", () => {
source.schema = undefined;
const {schema, shouldCoerce} = getSchema(source);
assert.strictEqual(shouldCoerce, true);
assert.deepStrictEqual(schema,[
{name: "a", type: "integer", inferred: "integer"},
{name: "b", type: "string", inferred: "string"}
]);
});

it("infers schema if schema is invalid", () => {
source.schema = ["number"];
const {schema, shouldCoerce} = getSchema(source);
assert.strictEqual(shouldCoerce, true);
assert.deepStrictEqual(schema,[
{name: "a", type: "integer", inferred: "integer"},
{name: "b", type: "string", inferred: "string"}
]);
});
});