Skip to content

Commit 0fa86e1

Browse files
authored
Fix: [Redshift] interval and numeric column type parsing (#27983)
* Fix: Redshift interval and numeric column type parsing * py_format * Fix numeric args fallback to _init_args when charlen is None * Add regression test for numeric(P, S) with space falling back to _init_args
1 parent 8dcf2a9 commit 0fa86e1

2 files changed

Lines changed: 111 additions & 3 deletions

File tree

ingestion/src/metadata/ingestion/source/database/redshift/utils.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,11 @@ def _get_kwargs_for_time_type(kwargs, charlen, attype):
234234
def _get_args_and_kwargs(charlen, attype, format_type):
235235
kwargs = {}
236236
args = _init_args(format_type)
237-
if attype == "numeric" and charlen:
238-
prec, scale = charlen.split(",")
239-
args = (int(prec), int(scale))
237+
if attype == "numeric":
238+
if charlen:
239+
args = tuple(int(p) for p in charlen.split(","))
240+
else:
241+
args = tuple(int(p) for p in args)
240242

241243
elif attype == "double precision":
242244
args = (53,)
@@ -257,6 +259,7 @@ def _get_args_and_kwargs(charlen, attype, format_type):
257259
args = (int(charlen),)
258260

259261
elif attype.startswith("interval"):
262+
args = ()
260263
field_match = re.match(r"interval (.+)", attype, re.I)
261264
if charlen:
262265
kwargs["precision"] = int(charlen)

ingestion/tests/unit/topology/database/test_redshift_utils.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,5 +368,110 @@ def test_numeric_and_character_varying_positional_arguments_are_unchanged(self):
368368
self.assertEqual(varchar_kwargs, {})
369369

370370

371+
class TestRedshiftIntervalParsing(unittest.TestCase):
372+
"""Test Redshift interval column type argument parsing."""
373+
374+
def test_interval_with_precision_uses_keyword_argument(self):
375+
"""interval(N) must route precision through kwargs, not positional args."""
376+
args, kwargs = _get_args_and_kwargs("6", "interval", "interval(6)")
377+
378+
self.assertEqual(args, ())
379+
self.assertEqual(kwargs, {"precision": 6})
380+
381+
coltype = _update_coltype(
382+
ischema_names["interval"],
383+
args,
384+
kwargs,
385+
"interval",
386+
"duration",
387+
False,
388+
)
389+
390+
self.assertEqual(coltype.precision, 6)
391+
self.assertIsNone(coltype.fields)
392+
393+
def test_interval_with_fields_and_precision_uses_keyword_arguments(self):
394+
"""interval <fields>(N) must route both precision and fields through kwargs."""
395+
args, kwargs = _get_args_and_kwargs("6", "interval day to second", "interval day to second(6)")
396+
397+
self.assertEqual(args, ())
398+
self.assertEqual(kwargs, {"precision": 6, "fields": "day to second"})
399+
400+
coltype = _update_coltype(
401+
ischema_names["interval"],
402+
args,
403+
kwargs,
404+
"interval",
405+
"duration",
406+
False,
407+
)
408+
409+
self.assertEqual(coltype.precision, 6)
410+
self.assertEqual(coltype.fields, "day to second")
411+
412+
def test_interval_without_precision_keeps_args_empty(self):
413+
"""Bare interval must produce empty args and empty kwargs."""
414+
args, kwargs = _get_args_and_kwargs(None, "interval", "interval")
415+
416+
self.assertEqual(args, ())
417+
self.assertEqual(kwargs, {})
418+
419+
420+
class TestRedshiftNumericParsing(unittest.TestCase):
421+
"""Test Redshift numeric column type argument parsing."""
422+
423+
def test_numeric_with_precision_only_does_not_crash(self):
424+
"""numeric(N) without scale must parse precision-only without ValueError."""
425+
args, kwargs = _get_args_and_kwargs("10", "numeric", "numeric(10)")
426+
427+
self.assertEqual(args, (10,))
428+
self.assertEqual(kwargs, {})
429+
430+
coltype = _update_coltype(
431+
ischema_names["numeric"],
432+
args,
433+
kwargs,
434+
"numeric",
435+
"amount",
436+
False,
437+
)
438+
439+
self.assertEqual(coltype.precision, 10)
440+
self.assertIsNone(coltype.scale)
441+
442+
def test_numeric_with_precision_and_scale_unchanged(self):
443+
"""Regression: numeric(P,S) must continue to parse both as positional args."""
444+
args, kwargs = _get_args_and_kwargs("10,2", "numeric", "numeric(10,2)")
445+
446+
self.assertEqual(args, (10, 2))
447+
self.assertEqual(kwargs, {})
448+
449+
coltype = _update_coltype(
450+
ischema_names["numeric"],
451+
args,
452+
kwargs,
453+
"numeric",
454+
"amount",
455+
False,
456+
)
457+
458+
self.assertEqual(coltype.precision, 10)
459+
self.assertEqual(coltype.scale, 2)
460+
461+
def test_numeric_without_charlen_keeps_args_empty(self):
462+
"""Bare numeric must produce empty args and empty kwargs."""
463+
args, kwargs = _get_args_and_kwargs(None, "numeric", "numeric")
464+
465+
self.assertEqual(args, ())
466+
self.assertEqual(kwargs, {})
467+
468+
def test_numeric_with_space_after_comma_falls_back_to_init_args(self):
469+
"""numeric(P, S) with a space must still parse precision and scale."""
470+
args, kwargs = _get_args_and_kwargs(None, "numeric", "numeric(10, 2)")
471+
472+
self.assertEqual(args, (10, 2))
473+
self.assertEqual(kwargs, {})
474+
475+
371476
if __name__ == "__main__":
372477
unittest.main()

0 commit comments

Comments
 (0)