Skip to content

fix: FIR-45354 Add null suffix to Firebolt types #141

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 19, 2025
Merged

fix: FIR-45354 Add null suffix to Firebolt types #141

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cdbd9de
fix: FIR-45354 Add null suffix to Firebolt types
bogdantruta-firebolt May 14, 2025
5ba72a9
fix CR
bogdantruta-firebolt May 16, 2025
f9d1fc9
revert decimal precision from type value
bogdantruta-firebolt May 19, 2025
aa55d4e
add return null statement to mapped type
bogdantruta-firebolt May 19, 2025
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
81 changes: 39 additions & 42 deletions src/statement/dataTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,30 @@ const typeMapping = {
bytea: "bytea"
};

const COMPLEX_TYPE = /^(nullable|array)\((.+)\)( null)?/;
const STRUCT_TYPE = /^(struct)\((.+)\)/;
const NULLABLE_TYPE = /^(.+)( null)$/;
const DATETIME_TYPE = /datetime64(.+)/i;
const TIMESTAMP_TYPE = /timestamp_ext(.+)/i;
const DECIMAL_TYPE = /decimal(.+)/i;
const NUMERIC_TYPE = /numeric(.+)/i;

const getMappedType = (innerType: string) => {
const type = typeMapping[innerType as keyof typeof typeMapping];
if (type) {
return type;
const match = NULLABLE_TYPE.exec(innerType);
const type = match ? match[1] : innerType;
const nullableSuffix = match ? " null" : "";
const mappedType = typeMapping[type as keyof typeof typeMapping];
if (mappedType) {
return `${mappedType}${nullableSuffix}`;
}
if (
RegExp(/datetime64(.+)/i).exec(innerType) ||
RegExp(/timestamp_ext(.+)/i).exec(innerType)
) {
return typeMapping.timestamp;
if (RegExp(DATETIME_TYPE).exec(type) || RegExp(TIMESTAMP_TYPE).exec(type)) {
return `${typeMapping.timestamp}${nullableSuffix}`;
}
if (
RegExp(/decimal(.+)/i).exec(innerType) ||
RegExp(/numeric(.+)/i).exec(innerType)
) {
return typeMapping.decimal;
if (RegExp(DECIMAL_TYPE).exec(type) || RegExp(NUMERIC_TYPE).exec(type)) {
return `${typeMapping.decimal}${nullableSuffix}`;
}
};

const COMPLEX_TYPE = /^(nullable|array)\((.+)\)/;
const STRUCT_TYPE = /^(struct)\((.+)\)/;

const DATE_TYPES = withNullableTypes([
"pg_date",
"pgdate",
Expand Down Expand Up @@ -89,20 +91,30 @@ export const STRING_TYPES = withNullableTypes(["string", "text"]);

export const BYTEA_TYPES = withNullableTypes(["bytea"]);

//todo fix nullable types FIR-45354
export const getFireboltType = (type: string): string => {
const key = type.toLowerCase();
const match = key.match(COMPLEX_TYPE);
const match = RegExp(COMPLEX_TYPE).exec(key);
if (match) {
const [_, outerType, innerType] = match;
if (innerType.match(COMPLEX_TYPE)) {
return getFireboltType(innerType);
}
const mappedType = getMappedType(innerType);
return mappedType ? `${outerType}(${mappedType})` : key;
const [, outerType, innerType, nullable] = match;
const fireboltType = getFireboltType(innerType);

return fireboltType
? `${outerType}(${fireboltType})${nullable ?? ""}`
: key;
}
const mappedType = getMappedType(key);
return mappedType ?? key;
};

export const getInnerType = (type: string): string => {
const key = type.toLowerCase();
const match = RegExp(COMPLEX_TYPE).exec(key);
if (match) {
const [, , innerType] = match;
return getFireboltType(innerType);
}
const mappedType = getMappedType(key);
return mappedType || key;
return mappedType ?? key;
};

const trimElement = (element: string) =>
Expand Down Expand Up @@ -142,31 +154,16 @@ export const getStructTypes = (type: string): Record<string, string> => {
return {};
};

export const getInnerType = (type: string): string => {
const key = type.toLowerCase();
const match = key.match(COMPLEX_TYPE);
if (match) {
const [_, _outerType, innerType] = match;
if (innerType.match(COMPLEX_TYPE)) {
return getInnerType(innerType);
}
const mappedType = getMappedType(innerType);
return mappedType || innerType;
}
const mappedType = getMappedType(key);
return mappedType || key;
};

export const isByteAType = (type: string) => {
return BYTEA_TYPES.indexOf(type) !== -1;
};

export const isDateType = (type: string) => {
return DATE_TYPES.indexOf(type) !== -1 || type.match(/datetime64(.+)/i);
return DATE_TYPES.indexOf(type) !== -1 || RegExp(DATETIME_TYPE).exec(type);
};

export const isFloatType = (type: string) => {
return FLOAT_TYPES.includes(type) || type.match(/decimal(.+)/i);
return FLOAT_TYPES.includes(type) || RegExp(DECIMAL_TYPE).exec(type);
};

export const isNumberType = (type: string) => {
Expand Down
4 changes: 2 additions & 2 deletions src/statement/hydrateResponse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import {
isByteAType,
isDateType,
isNumberType,
getInnerType,
getStructTypes,
isStructType
isStructType,
getInnerType
} from "./dataTypes";
import { hydrateDate } from "./hydrateDate";

Expand Down
46 changes: 24 additions & 22 deletions test/integration/v2/fetchTypes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,15 @@ describe("test type casting on fetch", () => {
" true as col_boolean,\n" +
" null::bool as col_boolean_null,\n" +
" [1,2,3,4] as col_array,\n" +
// " null::array(int) as col_array_null,\n" +
" null::array(int) as col_array_null,\n" +
" '1231232.123459999990457054844258706536'::decimal(38, 30) as col_decimal,\n" +
// " null::decimal(38, 30) as col_decimal_null,\n" +
" null::decimal(38, 30) as col_decimal_null,\n" +
" 'abc123'::bytea as col_bytea,\n" +
" null::bytea as col_bytea_null,\n" +
" 'point(1 2)'::geography as col_geography,\n" +
" null::geography as col_geography_null,"
" null::geography as col_geography_null,\n" +
" [[1,2],[null,2],null]::array(array(int)) as col_arr_arr,\n" +
" null::array(array(int)) as col_arr_arr_null"
);
const { data, meta } = await statement.fetchResult();
const metaObjects = [
Expand All @@ -218,14 +220,15 @@ describe("test type casting on fetch", () => {
{ name: "col_boolean", type: "boolean" },
{ name: "col_boolean_null", type: "boolean null" },
{ name: "col_array", type: "array(int)" },
// { name: "col_array_null", type: "array(int) null" },
// { name: "col_decimal", type: "decimal(38, 30)" },
{ name: "col_array_null", type: "array(int) null" },
{ name: "col_decimal", type: "decimal" },
// { name: "col_decimal_null", type: "decimal(38, 30) null" },
{ name: "col_decimal_null", type: "decimal null" },
{ name: "col_bytea", type: "bytea" },
{ name: "col_bytea_null", type: "bytea null" },
{ name: "col_geography", type: "geography" },
{ name: "col_geography_null", type: "geography null" }
{ name: "col_geography_null", type: "geography null" },
{ name: "col_arr_arr", type: "array(array(int null) null)" },
{ name: "col_arr_arr_null", type: "array(array(int)) null" }
];
for (let i = 0; i < meta.length; i++) {
expect(meta[i]).toEqual(metaObjects[i]);
Expand Down Expand Up @@ -261,29 +264,27 @@ describe("test type casting on fetch", () => {
" true as col_boolean,\n" +
" null::bool as col_boolean_null,\n" +
" [1,2,3,4] as col_array,\n" +
// " null::array(int) as col_array_null,\n" +
" null::array(int) as col_array_null,\n" +
" '1231232.123459999990457054844258706536'::decimal(38, 30) as col_decimal,\n" +
// " null::decimal(38, 30) as col_decimal_null,\n" +
" null::decimal(38, 30) as col_decimal_null,\n" +
" 'abc123'::bytea as col_bytea,\n" +
" null::bytea as col_bytea_null,\n" +
" 'point(1 2)'::geography as col_geography,\n" +
" null::geography as col_geography_null,"
" null::geography as col_geography_null,\n" +
" [[1,2],[null,2],null]::array(array(int)) as col_arr_arr,\n" +
" null::array(array(int)) as col_arr_arr_null"
);
const { data } = await statement.streamResult();
const [meta] = await stream.once(data, "meta");
const metaObjects = [
{ name: "col_int", type: "int" },
// { name: "col_int_null", type: "int null" },
{ name: "col_int_null", type: "integer null" },
{ name: "col_int_null", type: "int null" },
{ name: "col_long", type: "long" },
// { name: "col_long_null", type: "long null" },
{ name: "col_long_null", type: "bigint null" },
{ name: "col_long_null", type: "long null" },
{ name: "col_float", type: "float" },
// { name: "col_float_null", type: "float null" },
{ name: "col_float_null", type: "real null" },
{ name: "col_float_null", type: "float null" },
{ name: "col_double", type: "double" },
// { name: "col_double_null", type: "double null" },
{ name: "col_double_null", type: "double precision null" },
{ name: "col_double_null", type: "double null" },
{ name: "col_text", type: "text" },
{ name: "col_text_null", type: "text null" },
{ name: "col_date", type: "date" },
Expand All @@ -295,14 +296,15 @@ describe("test type casting on fetch", () => {
{ name: "col_boolean", type: "boolean" },
{ name: "col_boolean_null", type: "boolean null" },
{ name: "col_array", type: "array(int)" },
// { name: "col_array_null", type: "array(int) null" },
// { name: "col_decimal", type: "decimal(38, 30)" },
{ name: "col_array_null", type: "array(int) null" },
{ name: "col_decimal", type: "decimal" },
// { name: "col_decimal_null", type: "decimal(38, 30) null" },
{ name: "col_decimal_null", type: "decimal null" },
{ name: "col_bytea", type: "bytea" },
{ name: "col_bytea_null", type: "bytea null" },
{ name: "col_geography", type: "geography" },
{ name: "col_geography_null", type: "geography null" }
{ name: "col_geography_null", type: "geography null" },
{ name: "col_arr_arr", type: "array(array(int null) null)" },
{ name: "col_arr_arr_null", type: "array(array(int)) null" }
];
for (let i = 0; i < meta.length; i++) {
expect(meta[i]).toEqual(metaObjects[i]);
Expand Down
42 changes: 36 additions & 6 deletions test/unit/hydrate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,31 @@
import { getInnerType } from "../../src/statement/dataTypes";
import { getFireboltType, getInnerType } from "../../src/statement/dataTypes";
import { getStructTypes } from "../../src/statement/dataTypes";

describe("getInnerType function", () => {
describe("getFireboltType function", () => {
it("should return the firebolt type for a nullable type", () => {
const type = "nullable(int)";
const result = getFireboltType(type);
expect(result).toBe("nullable(int)");
});

it("should return the firebolt type for an array type", () => {
const type = "array(int)";
const result = getFireboltType(type);
expect(result).toBe("array(int)");
});

it("should return the firebolt type for a nullable array type", () => {
const type = "array(int null) null";
const result = getFireboltType(type);
expect(result).toBe("array(int null) null");
});

it("should return the firebolt type for a nullable array integer type", () => {
const type = "array(integer null) null";
const result = getFireboltType(type);
expect(result).toBe("array(int null) null");
});

it("should return the inner type for a nullable type", () => {
const type = "nullable(int)";
const result = getInnerType(type);
Expand All @@ -14,15 +38,21 @@ describe("getInnerType function", () => {
expect(result).toBe("int");
});

it("should return the inner type for a nested nullable type", () => {
const type = "nullable(nullable(int))";
it("should return the inner type for a nullable array type", () => {
const type = "array(int null) null";
const result = getInnerType(type);
expect(result).toBe("int");
expect(result).toBe("int null");
});

it("should return the inner type for a nullable array integer type", () => {
const type = "array(integer null) null";
const result = getInnerType(type);
expect(result).toBe("int null");
});

it("should return the original type if no complex type is found", () => {
const type = "int";
const result = getInnerType(type);
const result = getFireboltType(type);
expect(result).toBe("int");
});
});
Expand Down