[sqlite] System.Data.SQLite: Incorrect implementation of SQLiteDataReader::GetSchemaTable()

2016-01-04 Thread Jann Roder
Well, it doesn't really matter what the documentation says. Fact is that it is 
currently broken. I can easily patch this on my end using an IDataReader 
implementation that forwards all calls except the GetSchemaTable method. It 
would be sad though if a high quality library like SQLite contained a flaw like 
this.

-Original Message-
From: sqlite-users-bounces at mailinglists.sqlite.org 
[mailto:sqlite-users-boun...@mailinglists.sqlite.org] On Behalf Of Joe 
Mistachkin
Sent: 04 January 2016 15:43
To: SQLite mailing list 
Subject: Re: [sqlite] System.Data.SQLite: Incorrect implementation of 
SQLiteDataReader::GetSchemaTable()


The docs you quote from are specific to the "SqlClient" ADO.NET implementation, 
which is used to communicate only with MS SQL Server.  Other providers are 
apparently free to have completely different semantics.

Sent from my iPhone

> On Jan 4, 2016, at 1:25 AM, Jann Roder  wrote:
> 
> Looking at MSDN the docs are not that fuzzy:
> https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqldata
> reader.getschematable(v=vs.110).aspx
> 
> for example:
> 
> AllowDBNull : Set if the consumer can set the column to a null value or if 
> the provider cannot determine whether the consumer can set the column to a 
> null value. Otherwise, not set. A column may contain null values, even if it 
> cannot be set to a null value.
> 
> IsKey: true: The column is one of a set of columns in the rowset that, taken 
> together, uniquely identify the row. The set of columns with IsKey set to 
> true must uniquely identify a row in the rowset. There is no requirement that 
> this set of columns is a minimal set of columns. This set of columns may be 
> generated from a base table primary key, a unique constraint or a unique 
> index.
> 
> Unfortunately this is written from the perspective of someone using the 
> dataset to insert data into the database, but still the current 
> implementation violates the above.
> 
> While I understand your concern about backward compatibility this is still 
> clearly a bug in my opinion. Loading data into a datatable form SQLite is 
> only garunateed to work if the query is of the form SELECT * FROM Table. In 
> all other cases it depends. I think there should at least be an option that 
> will make it work in all cases.
> 
> Jann
> 
> -Original Message-
> From: sqlite-users-bounces at mailinglists.sqlite.org 
> [mailto:sqlite-users-bounces at mailinglists.sqlite.org] On Behalf Of Joe 
> Mistachkin
> Sent: 31 December 2015 17:45
> To: 'SQLite mailing list' 
> Subject: Re: [sqlite] System.Data.SQLite: Incorrect implementation of 
> SQLiteDataReader::GetSchemaTable()
> 
> 
> Jann Roder wrote:
>> 
>> The implementation of GetSchemaTable() in SQLiteDataReader seems to 
>> be based on the properties of the columns used in the select.  
>> Obviously uniqueness, nullness and primary key properties do not 
>> transfer when only a subset of the columns is selected or combined 
>> with data from other tables.
> 
> The official MSDN docs are a bit fuzzy on these points:
> 
> 
> https://msdn.microsoft.com/en-us/library/system.data.idatareader.getsc
> hemata
> ble%28v=vs.110%29.aspx
> 
>> 
>> Unfortunately the .Net Datatable::Load() command uses the information 
>> returned by SQLiteDataReader::GetSchemaTable().  This means that the 
>> result of many queries cannot be loaded into a Datatable using the
>> Load() command because of non-nullable columns being null (due to a 
>> left join) or even wrong (the Datatable just drops "duplicate" rows 
>> when loading data). See the code below that illustrates the problem.
> 
> Yes, I've seen this behavior while tracking down several past issues.
> On a somewhat sunnier note, the built-in ADO.NET support classes seem 
> remarkably good at adapting to [some of] the of idiosyncrasies of the various 
> ADO.NET providers.  However, as you have seen, there are some inconsistencies.
> 
>> 
>> I'm not sure how other DB drivers solve this problem, but it seems to 
>> me the SQLite driver should stop trying to be clever and just always 
>> set the IsUnique and IsKey columns to false and also always allow 
>> null values.
> 
> Unfortunately, changing this method in such a fundamental way would not be 
> backwards compatible.  Also, in some cases, it can be quite difficult to 
> determine what the "correct" behavior should be.
> 
> --
> Joe Mistachkin
> 
> ___
> sqlite-users mailing list
> sqlite-users at mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
> 
> 
> 
> Winton Capital

[sqlite] System.Data.SQLite: Incorrect implementation of SQLiteDataReader::GetSchemaTable()

2016-01-04 Thread Joe Mistachkin

Jann Roder wrote:
> 
> Well, it doesn't really matter what the documentation says. 
> 

Unfortunately, the ADO.NET documentation is the primary source on how to
implement compatible providers.  Are you aware of better sources?

> 
> I can easily patch this on my end using an IDataReader implementation
> that forwards all calls except the GetSchemaTable method.
> 

That seems like a reasonably good solution.

>
> It would be sad though if a high quality library like SQLite contained a
> flaw like this.
> 

To clarify: the issue here is with System.Data.SQLite, not SQLite itself.

--
Joe Mistachkin



[sqlite] System.Data.SQLite: Incorrect implementation of SQLiteDataReader::GetSchemaTable()

2016-01-04 Thread Jann Roder
Looking at MSDN the docs are not that fuzzy:
https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqldatareader.getschematable(v=vs.110).aspx

for example:

AllowDBNull : Set if the consumer can set the column to a null value or if the 
provider cannot determine whether the consumer can set the column to a null 
value. Otherwise, not set. A column may contain null values, even if it cannot 
be set to a null value.

IsKey: true: The column is one of a set of columns in the rowset that, taken 
together, uniquely identify the row. The set of columns with IsKey set to true 
must uniquely identify a row in the rowset. There is no requirement that this 
set of columns is a minimal set of columns. This set of columns may be 
generated from a base table primary key, a unique constraint or a unique index.

Unfortunately this is written from the perspective of someone using the dataset 
to insert data into the database, but still the current implementation violates 
the above.

While I understand your concern about backward compatibility this is still 
clearly a bug in my opinion. Loading data into a datatable form SQLite is only 
garunateed to work if the query is of the form SELECT * FROM Table. In all 
other cases it depends. I think there should at least be an option that will 
make it work in all cases.

Jann

-Original Message-
From: sqlite-users-bounces at mailinglists.sqlite.org 
[mailto:sqlite-users-boun...@mailinglists.sqlite.org] On Behalf Of Joe 
Mistachkin
Sent: 31 December 2015 17:45
To: 'SQLite mailing list' 
Subject: Re: [sqlite] System.Data.SQLite: Incorrect implementation of 
SQLiteDataReader::GetSchemaTable()


Jann Roder wrote:
>
> The implementation of GetSchemaTable() in SQLiteDataReader seems to be
> based on the properties of the columns used in the select.  Obviously
> uniqueness, nullness and primary key properties do not transfer when
> only a subset of the columns is selected or combined with data from
> other tables.
>

The official MSDN docs are a bit fuzzy on these points:


https://msdn.microsoft.com/en-us/library/system.data.idatareader.getschemata
ble%28v=vs.110%29.aspx

>
> Unfortunately the .Net Datatable::Load() command uses the information
> returned by SQLiteDataReader::GetSchemaTable().  This means that the
> result of many queries cannot be loaded into a Datatable using the
> Load() command because of non-nullable columns being null (due to a
> left join) or even wrong (the Datatable just drops "duplicate" rows
> when loading data). See the code below that illustrates the problem.
>

Yes, I've seen this behavior while tracking down several past issues.
On a somewhat sunnier note, the built-in ADO.NET support classes seem 
remarkably good at adapting to [some of] the of idiosyncrasies of the various 
ADO.NET providers.  However, as you have seen, there are some inconsistencies.

>
> I'm not sure how other DB drivers solve this problem, but it seems to
> me the SQLite driver should stop trying to be clever and just always
> set the IsUnique and IsKey columns to false and also always allow null
> values.
>

Unfortunately, changing this method in such a fundamental way would not be 
backwards compatible.  Also, in some cases, it can be quite difficult to 
determine what the "correct" behavior should be.

--
Joe Mistachkin

___
sqlite-users mailing list
sqlite-users at mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users



Winton Capital Management Limited (?Winton?) is a limited company registered in 
England and Wales with its registered offices at 16 Old Bailey, London, EC4M 
7EG (Registered Company No. 3311531). Winton is authorised and regulated by the 
Financial Conduct Authority in the United Kingdom, registered as an investment 
adviser with the US Securities and Exchange Commission, registered with the US 
Commodity Futures Trading Commission and a member of the National Futures 
Association in the United States.

This communication, including any attachments, is confidential and may be 
privileged. This email is for use by the intended recipient only. If you 
receive it in error, please notify the sender and delete it. You should not 
copy or disclose all or any part of this email.

This email does not constitute an offer or solicitation and nothing contained 
in this email constitutes, and should not be construed as, investment advice. 
Prospective investors should request offering materials and consult their own 
advisers with respect to investment decisions and inform themselves as to 
applicable legal requirements, exchange control regulations and taxes in the 
countries of their citizenship, residence or domicile. Past performance is not 
indicative of future results.

Winton takes reasonable steps to ensure the accuracy and integrity of its 
communications, including emails. Howeve

[sqlite] System.Data.SQLite: Incorrect implementation of SQLiteDataReader::GetSchemaTable()

2016-01-04 Thread Joe Mistachkin

The docs you quote from are specific to the "SqlClient" ADO.NET implementation, 
which is used to communicate only with MS SQL Server.  Other providers are 
apparently free to have completely different semantics.

Sent from my iPhone

> On Jan 4, 2016, at 1:25 AM, Jann Roder  wrote:
> 
> Looking at MSDN the docs are not that fuzzy:
> https://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqldatareader.getschematable(v=vs.110).aspx
> 
> for example:
> 
> AllowDBNull : Set if the consumer can set the column to a null value or if 
> the provider cannot determine whether the consumer can set the column to a 
> null value. Otherwise, not set. A column may contain null values, even if it 
> cannot be set to a null value.
> 
> IsKey: true: The column is one of a set of columns in the rowset that, taken 
> together, uniquely identify the row. The set of columns with IsKey set to 
> true must uniquely identify a row in the rowset. There is no requirement that 
> this set of columns is a minimal set of columns. This set of columns may be 
> generated from a base table primary key, a unique constraint or a unique 
> index.
> 
> Unfortunately this is written from the perspective of someone using the 
> dataset to insert data into the database, but still the current 
> implementation violates the above.
> 
> While I understand your concern about backward compatibility this is still 
> clearly a bug in my opinion. Loading data into a datatable form SQLite is 
> only garunateed to work if the query is of the form SELECT * FROM Table. In 
> all other cases it depends. I think there should at least be an option that 
> will make it work in all cases.
> 
> Jann
> 
> -Original Message-
> From: sqlite-users-bounces at mailinglists.sqlite.org 
> [mailto:sqlite-users-bounces at mailinglists.sqlite.org] On Behalf Of Joe 
> Mistachkin
> Sent: 31 December 2015 17:45
> To: 'SQLite mailing list' 
> Subject: Re: [sqlite] System.Data.SQLite: Incorrect implementation of 
> SQLiteDataReader::GetSchemaTable()
> 
> 
> Jann Roder wrote:
>> 
>> The implementation of GetSchemaTable() in SQLiteDataReader seems to be
>> based on the properties of the columns used in the select.  Obviously
>> uniqueness, nullness and primary key properties do not transfer when
>> only a subset of the columns is selected or combined with data from
>> other tables.
> 
> The official MSDN docs are a bit fuzzy on these points:
> 
> 
> https://msdn.microsoft.com/en-us/library/system.data.idatareader.getschemata
> ble%28v=vs.110%29.aspx
> 
>> 
>> Unfortunately the .Net Datatable::Load() command uses the information
>> returned by SQLiteDataReader::GetSchemaTable().  This means that the
>> result of many queries cannot be loaded into a Datatable using the
>> Load() command because of non-nullable columns being null (due to a
>> left join) or even wrong (the Datatable just drops "duplicate" rows
>> when loading data). See the code below that illustrates the problem.
> 
> Yes, I've seen this behavior while tracking down several past issues.
> On a somewhat sunnier note, the built-in ADO.NET support classes seem 
> remarkably good at adapting to [some of] the of idiosyncrasies of the various 
> ADO.NET providers.  However, as you have seen, there are some inconsistencies.
> 
>> 
>> I'm not sure how other DB drivers solve this problem, but it seems to
>> me the SQLite driver should stop trying to be clever and just always
>> set the IsUnique and IsKey columns to false and also always allow null
>> values.
> 
> Unfortunately, changing this method in such a fundamental way would not be 
> backwards compatible.  Also, in some cases, it can be quite difficult to 
> determine what the "correct" behavior should be.
> 
> --
> Joe Mistachkin
> 
> ___
> sqlite-users mailing list
> sqlite-users at mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users
> 
> 
> 
> Winton Capital Management Limited (?Winton?) is a limited company registered 
> in England and Wales with its registered offices at 16 Old Bailey, London, 
> EC4M 7EG (Registered Company No. 3311531). Winton is authorised and regulated 
> by the Financial Conduct Authority in the United Kingdom, registered as an 
> investment adviser with the US Securities and Exchange Commission, registered 
> with the US Commodity Futures Trading Commission and a member of the National 
> Futures Association in the United States.
> 
> This communication, including any attachments, is confidential and may be 
> privileged. This email is for use by the intend

[sqlite] System.Data.SQLite: Incorrect implementation of SQLiteDataReader::GetSchemaTable()

2015-12-31 Thread Jann Roder
Hi,
The implementation of GetSchemaTable() in SQLiteDataReader seems to be based on 
the properties of the columns used in the select. Obviously uniqueness, 
nullness and primary key properties do not transfer when only a subset of the 
columns is selected or combined with data from other tables. Unfortunately the 
.Net Datatable::Load() command uses the information returned by
SQLiteDataReader::GetSchemaTable(). This means that the result of many queries 
cannot be loaded into a Datatable using the Load() command because of 
non-nullable columns being null (due to a left join) or even wrong (the 
Datatable just drops "duplicate" rows when loading data). See the code below 
that illustrates the problem.

I'm not sure how other DB drivers solve this problem, but it seems to me the 
SQLite driver should stop trying to be clever and just always set the IsUnique 
and IsKey columns to false and also always allow null values.

Jann

using System.Data;
using System.Data.SQLite;
using NUnit.Framework;

namespace SQLiteDemo
{
[TestFixture]
public class Demo
{
private SQLiteConnection m_connection;

[SetUp]
public void CreateDatabase()
{
m_connection = new SQLiteConnection("Data Source=:memory:");
m_connection.Open();
using (var command = new SQLiteCommand())
{
command.CommandText =
@"CREATE TABLE P (
  Pid1 INT NOT NULL,
  Pid2 INT NOT NULL,
  Value TEXT NOT NULL,
  CONSTRAINT PK_P PRIMARY KEY (Pid1, Pid2)
  );
CREATE TABLE PM (
  Pid1 INT NOT NULL,
  Pid2 INT NOT NULL,
  Value TEXT NOT NULL,
  CONSTRAINT PK_PM PRIMARY KEY (Pid1, Pid2, Value)
  );";
command.Connection = m_connection;
command.ExecuteNonQuery();
}

using (var command = new SQLiteCommand())
{
command.CommandText =
@"INSERT INTO P VALUES(1, 1, 'A');
  INSERT INTO P VALUES(2, 1, 'A');
  INSERT INTO P VALUES(3, 1, 'B');
INSERT INTO PM VALUES(1, 10, 'A');
INSERT INTO PM VALUES(3, 1, 'A');";
command.Connection = m_connection;
command.ExecuteNonQuery();
}
}

[TearDown]
public void CleanDatabase()
{
m_connection.Close();
m_connection = null;
}

[Test]
public void Case1()
{
var dt = new DataTable();
using (var command = new SQLiteCommand(@"SELECT P.Pid1, PM.Pid2
  FROM P
  LEFT JOIN PM ON P.Value = 
PM.Value", m_connection))
{
using (var reader = command.ExecuteReader())
{
var schema = reader.GetSchemaTable();

dt.Load(reader);
}
}

Assert.That(dt.Rows.Count, Is.EqualTo(5));
}

[Test]
public void Case2()
{
var dt = new DataTable();
using (var command = new SQLiteCommand(@"SELECT P.Pid1,
 CASE WHEN PM.Pid2 IS NULL 
THEN P.Pid2 ELSE PM.Pid2 END
  FROM P
  LEFT JOIN PM ON P.Value = 
PM.Value", m_connection))
{
using (var reader = command.ExecuteReader())
{
var schema = reader.GetSchemaTable();

dt.Load(reader);
}
}

Assert.That(dt.Rows.Count, Is.EqualTo(5));
}
}
}



Winton Capital Management Limited (?Winton?) is a limited company registered in 
England and Wales with its registered offices at 16 Old Bailey, London, EC4M 
7EG (Registered Company No. 3311531). Winton is authorised and regulated by the 
Financial Conduct Authority in the United Kingdom, registered as an investment 
adviser with the US Securities and Exchange Commission, registered with the US 
Commodity Futures Trading Commission and a member of the National Futures 
Association in the United States.

This communication, including any attachments, is confidential and may be 
privileged. This email is for use by the intended recipient only. If you 
receive it in error, please notify the sender and delete it. You should not 
copy or disclose all or any part of this email.

This email does not constitute an offer or solicitation and nothing contained 
in this email constitutes, and should not be construed as, investment advice. 
Prospective investors should request offering 

[sqlite] System.Data.SQLite: Incorrect implementation of SQLiteDataReader::GetSchemaTable()

2015-12-31 Thread Joe Mistachkin

Jann Roder wrote:
>
> The implementation of GetSchemaTable() in SQLiteDataReader seems to be
> based on the properties of the columns used in the select.  Obviously
> uniqueness, nullness and primary key properties do not transfer when
> only a subset of the columns is selected or combined with data from
> other tables. 
>

The official MSDN docs are a bit fuzzy on these points:


https://msdn.microsoft.com/en-us/library/system.data.idatareader.getschemata
ble%28v=vs.110%29.aspx

> 
> Unfortunately the .Net Datatable::Load() command uses the information
> returned by SQLiteDataReader::GetSchemaTable().  This means that the
> result of many queries cannot be loaded into a Datatable using the
> Load() command because of non-nullable columns being null (due to a
> left join) or even wrong (the Datatable just drops "duplicate" rows
> when loading data). See the code below that illustrates the problem.
> 

Yes, I've seen this behavior while tracking down several past issues.
On a somewhat sunnier note, the built-in ADO.NET support classes seem
remarkably good at adapting to [some of] the of idiosyncrasies of the
various ADO.NET providers.  However, as you have seen, there are some
inconsistencies.

> 
> I'm not sure how other DB drivers solve this problem, but it seems to
> me the SQLite driver should stop trying to be clever and just always
> set the IsUnique and IsKey columns to false and also always allow null
> values.
> 

Unfortunately, changing this method in such a fundamental way would not
be backwards compatible.  Also, in some cases, it can be quite difficult
to determine what the "correct" behavior should be.

--
Joe Mistachkin