feat(mssql): add lpad and rpad ops

[IndexSeek]

Description of changes

I noticed that MSSQL was the only backend missing lpad and rpad, so I wanted to add support for these.

The LPAD operation was a bit tricky. I'm wondering if there might be a more straightforward way, but after several attempts, this seemed to handle all of the cases well.

[IndexSeek]

Here was my initial implementation before it became what it is now:

    def visit_LPad(self, op, *, arg, length, pad=" "):
        return self.f.right(self.f.concat(self.f.replicate(pad, length), arg), length)

But it was giving me this result:

string_col	index_col	LPad(string_col, 4, '-')
AbC\t	0	AbC\t
\n123\n	1	\n
abc, 123	2	123
123	3	-123
aBc	4	-aBc
🐍	5	---🐍
ÉéÈèêç	6	Èèêç

Where I was expecting this result:

string_col	index_col	LPad(string_col, 4, '-')
AbC\t	0	AbC\t
\n123\n	1	\n123
abc, 123	2	abc,
123	3	-123
aBc	4	-aBc
🐍	5	---🐍
ÉéÈèêç	6	ÉéÈè

[cpcloud]

Thanks for working through this!

[cpcloud]

@IndexSeek The pad functions are not consistent in their truncation behavior right now, so this PR should handle that at least for MS SQL.

The desired behavior is the Python behavior, which is to never truncate if the padded string would be shorter than existing string.

For example:

>>> s = "abc, 123"
>>> s.ljust(4, '-')
'abc, 123'

[cpcloud]

Added @gforsyth for review since he's been doing a bunch of work trying to vanquish (or least contain) the dark underbelly of accursed database strings.

[IndexSeek]

The desired behavior is the Python behavior, which is to never truncate if the padded string would be shorter than existing string.

For example:

>>> s = "abc, 123"
>>> s.ljust(4, '-')
'abc, 123'

That makes total sense! I have an idea (that involves a case expression) of how we can make this work, and I'll try to tackle it this evening. Thanks for the initial review @cpcloud and the quick fix on the default value!

Added @gforsyth for review since he's been doing a bunch of work trying to vanquish (or least contain) the dark underbelly of accursed database strings.

Thanks for looping @gforsyth in! Containing the strings is no easy feat! When I looked through the tests, I thought the following section would make a great candidate for addition to the new "Cursed Knowledge" page:

ibis/ibis/backends/tests/test_string.py

Lines 1143 to 1152 in 3533827

	pytest.mark.notyet(
	["flink", "oracle"],
	raises=AssertionError,
	reason="Treats len(🐍) == 2 so padding is off",
	),
	pytest.mark.notyet(
	["impala"],
	raises=AssertionError,
	reason="Treats len(🐍) == 4, len(Éé) == 4",
	),

[cpcloud]

Oh, yeah, that page is just getting started 😂

[gforsyth]

Oh, yeah, that page is just getting started 😂

It will grow without bound

[gforsyth]

I think I'll add a few more (technically redundant) test tables for padding and length (don't let this block you here @IndexSeek) so we can more explicitly map out "levels" of string support into things like:

1. Python style padding for "simple" strings (ascii-like)
2. Python style padding for basic unicode (accented letters, non-english alphabets)
3. Python style padding for graphemes, treating emoji as single characters

[IndexSeek]

I have adjusted the functions now to support Python-style padding. I was able to reuse much of the previous logic with a slight change, essentially:

if the length in the l/rpad argument is greater than or equal to the length of the arg being padded:
    return the arg being padded
else:
    return the original expression

Since this behavior differs from the other backends, I added "mssql" to the notyet section for reason="Python style padding, e.g. doesn't trim strings to pad-length". I hope this will work in the meantime!

[gforsyth]

This looks good to me, @IndexSeek ! As noted, I have some new string data that's a little more simple-ASCII for validating the simpler lpad and rpad cases and your implementation here matches Python's padding behavior, modulo issues with the unclear definition of what "length" is.

[IndexSeek]

This looks good to me, @IndexSeek ! As noted, I have some new string data that's a little more simple-ASCII for validating the simpler lpad and rpad cases and your implementation here matches Python's padding behavior, modulo issues with the unclear definition of what "length" is.

Thank you for the review! "length" is definitely tricky here.

SQL Server/T-SQL has the DATALENGTH function, returning the number of bytes, where as it's LEN function returns the number of characters. But it seems that some backends treat LEN like DATALENGTH. ¯\__(ツ)__/¯

I am excited to review the new string data - it will be great to cover more cases!