ADODB_DataDict._GenFields: Unconditional remove of nullability and default on blobs

[obmsch]

In 'adodb-datadict' inside _GenFields:

line 729
if ($ty == 'X' || $ty == 'X2' || $ty == 'B') $fnotnull = false; // some blob types do not accept nulls [cf #291]

line 733-734
//some databases do not allow blobs to have defaults
if ($ty == 'X') $fdefault = false;

Thats completely ignoring the capabilites of any db to honor some dbs not implementing that feature. I am looking for a better approach(My knowledge of php and its inner workings is really limited. So this is more conceptional than a real implementation).

Add members (for capabilities) to the base class ADODB_DataDict like
$hasNullableBlobs = false;
$hasDefaultOnBlobs = false;

Change the code above to:
if (!$this->hasNullableBlobs && ($ty == 'X' || $ty == 'X2' || $ty == 'B')) $fnotnull = ...
and
if (!$this->hasDefaultOnBlobs && $ty == 'X') $fdefault = false;

Thats a noop in behaviour of ADOdb.

But lets say a db supports NULL in blobs, setting $hasNullableBlobs = true; in its datadict should do the trick.

[obmsch]

Missed an outstanding pull request pr #118, adressing this issue.

Reconsidering my first thougts and the approach taken in the pull request, I would now recommend to handle this in a more general way in the generic datadict. There might be other field types beside blobs with restrictions too.

And if we agree on:

Any given schema should be me mapped as precisely as possible to the underlying db.

then

Either we drop those lines completely and let it to the dbs datadict to handle this, implementing its own _GenFields or take care of it in Add/AlterColumnSQL (caveat: to preserve current behaviour all dbs datadicts have to be adjusted, and at least me don't know what all the supported dbs would require then).

or

a) Extend the generic datadict with something like:

  HasNull($t) {
    return true;
  }
  HasDefault($t) {
    return true;
  }

b) Alter the corresponding lines:

if (!this->HasNull($ty)) {
   $fnotnull = false;
}

if (!this->HasDefault($ty)) {
   $fdefault = false;
}

c) To preserve current behaviour (and put it to the db to handle) extend all dbs datadicts with:

HasNull($t) {
  case 'X':
  case 'X2':
  case 'B' : return false;
  default: return true;
}
  
HasDefault($t) {
  case 'X' : return false;
  default: return true;
}

d) Adjust (c) for the known ones (datadict_?) in the first place [1]:

mysql (from pr #118)

  HasNull($t) {
    return true;
  }

postgres (from pr #118)

  HasNull($t) {
    return true;
  }
  HasDefault($t) {
    return true;
  }

mssql (me)

  HasNull($t) {
    return true;
  }
  HasDefault($t) {
    return true;
  }

If this sounds reasonable to the devs and we could agree on d)+[1] I would volunteer a pull request on this. But given the hassle I have had with the last one (git with me and my dev box 👎 ), only if the path we want to go is set.

[1] Correct/Add others.

[dregad]

@obmsch thank you for your follow-up and proposal to volunteer. Due to my current workload I don't cant look at this into details ATM, will do so as soon as I can (no promises on when).

As a side note, if you need any help with Git, feel free to contact me on Gitter at any time though.