A small fix and upcoming PR for SQLServer fixes

[jr3cermak]

~line 2264:

		foreach ($tables as $t=>$table)	{
			$table_list = array($table['name']);
			$table_fields = $this->findFields($table_list,false,false,false,$database);
			$table_names = array_map(function($v){ return $v['name'];},$tables);

It looks like:
$table_names = array_map(function($v){ return $v['name'];},$tables);

is run multiple times over the foreach loop, could this be safely moved to just before the foreach loop? If so, I can include it in a future PR.

I am also working on a PR for some fixes to the SQLServer driver. See: jr3cermak#2

Quick summary:

• Swagger supports a boolean type that SQLServer defines as type -7(bit) currently shown as string. We might add a isBooleanType check across the board.
• SQLserver type 3 is numeric.
• sqlsrv_field_metadata may return NULL for a requested table. Although the table may exist or show via information_schema, we may want to eliminate this table from being shown. Otherwise, the columns are shown as strings which is wrong.
• don't show maxLength < 0

[mevdschee]

Thank you for contribution and constructive discussion. I have remarks and questions.

is run multiple times over the foreach loop

You are right, this could be moved out of the loop, good catch!

I am also working on a PR for some fixes to the SQLServer driver

Great! I use the SQL Server driver myself and am very interested in improvements.

support a boolean type

1. This feature is not SQL Server specific
2. SQL does not recognize boolean as a type.
3. We could map "bit" (comes close), but that is broken in mysql.

Adding types costs performance, maybe we should do something to mitigate the costs (caching?).

SQLserver type 3 is numeric.

This is intentional. Decimal (3) is often used for money (although it is probably better to use integers for money) and money should not be stored as a float.

See: https://docs.microsoft.com/en-us/sql/connect/php/sqlsrv-field-metadata

sqlsrv_field_metadata may return NULL for a requested table

When does that happen?

don't show maxLength < 0

When does that happen?

[jr3cermak]

For type 3, the field shows up as "Number" in MS Access. Unfortunately, float is not an available type in the schema object so we are left with number. Which is why we are passing x-dbtype.

maxLength is shown as -1 for fields defined as VARCHAR(MAX) or VARBINARY(MAX). (https://blog.sqlauthority.com/2012/07/20/sql-server-information_schema-columns-and-value-character-maximum-length-1/). For these, the documentation says max length is 8000, but I'd rather that come back from the db server. Do we need to offer a x-dbmax?

If a table has a field defined like above, the current api shows which swagger rejects.

              definition:
                type: string
                format: byte
                x-dbtype: varbinary
                x-nullable: true
                maxLength: -1

What MS Access calls a Memo field is shown like this which is nice.

ClassNotes: {
type: "string",
x-dbtype: "ntext",
x-nullable: true,
maxLength: 1073741823
},

I don't have access to a test database that I can blow away or I would add a test case.

With the fix on sqlsrv_field_metadata, the table that produced that error does not show up now. I am also playing with a SQLServer database, but I don't have admin access. I have two separate logins however that have varying permissions on the tables. I have one account that I can use for debugging and another for the api which has reduced access to tables. With the reduced access, I see cases where sqlsrv_field_metadata() returns NULL but some other information is available via information_schema.

To reproduce the problem, reduce table permissions for the user.

Why do this? The DB admin can then control access via the DB and not have to mess with the api code. I don't want to have to mess with the api code once it is setup unless I really have too.

[mevdschee]

Unfortunately, float is not an available type in the schema object so we are left with number.

We should return the value of a decimal type as a string in JSON (a number with decimals between quotes) and the "type" in swagger should reflect that by showing "string".

{
    "id":1,
    "name":"calculator",
    "price":"12.95"
}

I feel the swagger "type" field should reflect the expected JSON type, not the database type. In "x-dbtype" we should reflect the type in the database, which is "decimal" (and thus numeric).

maxLength is shown as -1 for fields defined as VARCHAR(MAX) or VARBINARY(MAX).

The "maxLength" of "-1" seems to mean that there is no explicitly defined maxlength and I propose to leave out the maxLength property in that case.

sqlsrv_field_metadata() returns NULL.. reduce table permissions for the user.

Would the table be listed in that case in the "tables" view of the "information schema"? I'm not sure catching NULL in the metadata is the best way to determine permissions on tables.

The DB admin can then control access via the DB and not have to mess with the api code.

I completely understand this need and agree that this is desirable.

[jr3cermak]

{
    "id":1,
    "name":"calculator",
    "price":"12.95"
}

I feel the swagger "type" field should reflect the expected JSON type, not the database type. In "x-dbtype" we should reflect the type in the database, which is "decimal" (and thus numeric).

It may be ok for GET.

By doing that, does that also effect the INSERT and UPDATE methods for PUT and PATCH? If the swagger advertises 'string' then the SQL created by swagger code generation will generate a string for input?

[mevdschee]

By doing that, does that also effect the INSERT and UPDATE methods for PUT and PATCH?

Yes, it does.

If the swagger advertises 'string' then the SQL created by swagger code generation will generate a string for input?

Yes. This does, in my experience, not cause any problems (unlike using floats for money).

[mevdschee]

The request to support boolean is now covered by #218

[mevdschee]

It seems that you have 'Limited metadata accessibility' in your database, as described here:

https://msdn.microsoft.com/en-us/library/ms187113.aspx

Could it be that with stricter permissions the table is not even listed (as intended)?

[mevdschee]

I believe I covered/answered all different requests in this issue and the related PR. Please open a new issue if you want to continue to talk about something specific.