Fix #15093: Add additional field type metadata for pdo_mysql in getColumnMeta response.

[deAtog]

This fixes #15093 by checking for and adding additional flags to the output from getColumnMeta. It also adds a 'native_flags' attribute that returns the unmodified flags from mysql should the flags be expanded in the future.

[devnexen]

You need to update/fix the following test

========DIFF========
--
         [flags] => Array
             (
                 [0] => not_null
016+             [1] => no_default_value
017+             [2] => num
             )
     
020+     [native_flags] => 36865
         [table] => test_33689
         [name] => bar
         [len] => 11
--
========DONE========
FAIL PDO MySQL Bug #33689 (query() execute() and fetch() return false on valid select queries) [ext/pdo_mysql/tests/bug_33689.phpt] 

maybe also adding a new test to demonstrate the full extent of your proposal, what do you think ?

[SakiTakamachi]

Sorry for the late reply, I was not feeling well. I'll check this later.

[devnexen]

note: that or that might be ways around it. e.g. sorting. even for the type display, you re using EXPECTF after all.

[deAtog]

This section was pulled from another test. As stated in the comment, the value of the pdo_type may differ based on which driver is used. I only modify the flags and added a native_flags attribute, which is really what the test is meant to cover.

[devnexen]

fair enough, anyhow the maintainer will review all properly soon :)

[SakiTakamachi]

Is it possible to split the tests per driver and check the value of pdo_type in the test to ensure I am not breaking existing functionality?
These behaviors don't change from driver to driver, just the values ​​change, so I think it's okay to just check it with mysqlnd testing.

[SakiTakamachi]

IS_NUM condition has changed. What impact does the removed condition have?
In other words, what happens in the new logic if the value is FIELD_TYPE_YEAR or FIELD_TYPE_NEWDECIMAL?

[deAtog]

IS_NUM was declared, but never used by mysqlnd. I suppose this would be a breaking change though for anything that imports this header file externally and uses this definition. Internally, mysqlnd sets the NUM_FLAG under the following conditions:

php-src/ext/mysqlnd/mysqlnd_wireprotocol.c

Lines 1221 to 1228 in 0e33b8d

	/* Should we set NUM_FLAG (libmysql does it) ? */
	if (
	(meta->type <= MYSQL_TYPE_INT24 &&
	(meta->type != MYSQL_TYPE_TIMESTAMP || meta->length == 14 || meta->length == 8)
	) || meta->type == MYSQL_TYPE_YEAR)
	{
	meta->flags |= NUM_FLAG;
	}

However, MySql sets the NUM_FLAG based on their definition of IS_NUM here:
https://github.com/mysql/mysql-server/blob/596f0d238489a9cf9f43ce1ff905984f58d227b6/include/mysql.h#L116-L118

Maybe the condition in mysqlnd should be changed to use the condition provided by mysql?

[SakiTakamachi]

MySQLPDOTest::skipNotMySQLnd();

if (MYSQLPDOTest::isPDOMySQLnd()) die("skip libmysql only test");

By using the above codes, you can separate the tests for mysqlnd and libmysql.
IMHO, these features are less common and less likely to be bug-reported, so it is desirable to be able to cover all cases with tests.

[deAtog]

I changed the code to use skipNotMySQLnd(), but in retrospect I think you were suggesting to write two separate tests. Given the differences regarding the NUM_FLAG above, separating them does seem like a good idea at first. However, one would expect the two to be functionally equivalent in this test.