SCM

[#1000458] Enormous memory increase (and application crash) with large BYTEA parameter

View Trackers | Bugs | Download .csv | Monitor

Date:
2005-11-30 17:42
Priority:
3
State:
Closed
Submitted by:
Nobody
Assigned to:
Nobody (None)
Npgsql Version:
None
Category:
Group:
Resolution:
Accepted
Summary:
Enormous memory increase (and application crash) with large BYTEA parameter

Detailed description
Hi,

Environment:

Microsoft Windows XP Pro SP2
PostgreSQL 8.0.4 on Linux Gentoo
.Net Framework 1.1
Npgsql-0.7.1 assembly.

I have an application that inserts some large objects in a BYTEA column. Those objects are passed as parameters of the command.
When executing the command.ExecuteNonQuery() the memory of my machine increases dramatically, for instance up to 600 Mb RAM for a parameter with a 22 Mb pdf file.

Here follows the csharp code of a test case :

First execute the CreateTable(string) method then FillTable(string) that crashes.

Web.config : one should add
<appSettings>
<add key="cnxstring" value="Server=myserver;Port=5432;User Id=myuser;Password=mypassword;Database=mydatabase;SSL=false;" />
<add key="DocumentFolder" value="C:\docs\" />
</appSettings>

and a large file in the folder DocumentFolder named 'thesis.pdf'.

using System;
using System.Collections;
using System.Configuration;
using System.ComponentModel;
using System.Data;
using System.IO;
using Npgsql;
using NpgsqlTypes;

/ ... /

private void CreateTable(string cnxString)
{
NpgsqlConnection pgcnx = new NpgsqlConnection(cnxString);
NpgsqlCommand pgcmd = new NpgsqlCommand();
pgcmd.Connection = pgcnx;
pgcmd.CommandText = @" CREATE TABLE MyTable (
Unique_Id VARCHAR(36) NOT NULL,
Text VARCHAR(100),
Value BYTEA,
CONSTRAINT PK_MyTable_Unique_Id PRIMARY KEY (Unique_Id)
) ";
pgcnx.Open();
try
{
pgcmd.ExecuteNonQuery();
}
catch(Npgsql.NpgsqlException pgExc)
{
throw pgExc;
}
finally
{
pgcnx.Close();
}
}

private void FillTable(string cnxString)
{
ArrayList list = new ArrayList();
list.Add("William");
NpgsqlConnection pgcnx = new NpgsqlConnection(cnxString);
NpgsqlCommand pgcmd = new NpgsqlCommand();
pgcmd.Connection = pgcnx;
pgcnx.Open();
foreach (string s in list)
{
pgcmd.Parameters.Clear();
pgcmd.CommandText = @" INSERT INTO MyTable (Unique_Id, Text, Value) VALUES (:punique_Id, :ptext, :pvalue) ";
(pgcmd.Parameters.Add("punique_Id",NpgsqlDbType.Text)).Value = Guid.NewGuid().ToString();
(pgcmd.Parameters.Add("ptext",NpgsqlDbType.Text)).Value = s;
(pgcmd.Parameters.Add("pvalue",NpgsqlDbType.Bytea)).Value = ToByteArray("thesis.pdf"); // 22 Mbytes pdf file.
try
{
pgcmd.ExecuteNonQuery();
}
catch(Npgsql.NpgsqlException pgExc)
{
throw pgExc;
}
finally
{
pgcnx.Close();
}
}
}

internal static byte[] ToByteArray(string fileName)
{
FileStream fsInput = new FileStream(ConfigurationSettings.AppSettings["DocumentFolder"].ToString()+fileName, FileMode.Open, FileAccess.Read);
byte[] bytearrayinput = new byte[fsInput.Length];
fsInput.Read(bytearrayinput, 0, bytearrayinput.Length);
return bytearrayinput;
}

Thanks in advance for your help.
Note : i attached a zip of the Visual Studio project to the message.

Arnaud Balandras

Followup

Message
Date: 2006-02-25 16:06
Sender: Francisco Figueiredo jr.


Patch fixed in cvs.

Thanks for your feedback, Hubert!
Date: 2005-12-14 16:24
Sender: Francisco Figueiredo jr.


Just a note:

I applied the patch to pgfoundry cvs, not Mono svn. I will apply to Mono svn soon. Could you confirm first if it worked for you?

Also, it seems that Mono guys are preparing for a 1.1.11 release. Maybe we should wait for this release before applying any other patches.

Thanks in advance.
Date: 2005-12-14 16:21
Sender: Francisco Figueiredo jr.

Hi Hubert.

Patch applied, finally :)

I was playing with some performance improvements I talked about in my blog and you correctly noticed that it didn't make a big difference. The fact was that I had to tweak a little bit the Stream.Flush() calls which were removing the gain of BufferedStream by writing to network more times than necessary :)

But now it is done. Please, give it a try and let me know if it worked for you.

Thank you very much for patch and feedback.
Date: 2005-12-14 00:29
Sender: Francisco Figueiredo jr.


Hi Hubert.

I did the modification and it worked ok.
I'm now just doing some last performance checks with some code I added to commit your patch.
Date: 2005-12-13 20:06
Sender: Hubert FONGARNAND

"About the prepare call inside the executereader", I think about that, because all other database connector (oracle, sql server) do this... and because if you don't use the prepare method, the performance is always awful with binary parameter... (and developpers could forget to call this method).

I'll see too (maybe tomorrow) if sending the describe call when we create the bind object makes it work ok...
Date: 2005-12-13 19:07
Sender: Francisco Figueiredo jr.


I got an answer from Tom Lane from Postgresql with a patch for it. Indeed it was a problem with Postgresql. :)

I will try to see if changing the describe call to where we create the bind object makes it work ok. If not, we will have to add a note about this problem with Postgresql when using prepared statements with postgresql.
Date: 2005-12-13 16:03
Sender: Francisco Figueiredo jr.


Hi Hubert.

Your patch was very good! And I think it showed a problem in postgresql server itself. :) Postgresql shouldn't be behaving like it is now. In no way I think your patch is bad. Sorry if I made you interpret it was. It was not my intention.


About the prepare call inside the executereader, it is doable but I think it would hurt a lot the performance for single calls. That's why I tried the prepare and executereader;executereader; I think you would prepare the call one time and later would just change values or call the same query many times.

This way, it would up to user to decide when to prepare or not a call.

But we can make tests to see if performance is so much hurt. :( Maybe it is not :)



Date: 2005-12-13 08:28
Sender: Hubert FONGARNAND

It don't fails if you call

NpgsqlCommand c = new NpgsqlCommand("a select
command");
c.Prepare();
c.ExecuteReader();
c.Prepare();
c.ExecuteReader();

i think you should include the call to c.Prepare to the ExecuteReader command!
Date: 2005-12-13 08:24
Sender: Hubert FONGARNAND

I've done the test too... it crashes too. I'm using PostGreSQL 8.0.4. I think it's a bug of postgresql.
I think it will be better to send the describe command when creating NpgsqlBind!

(my patch was just a try... and is not perfect at all!)
Date: 2005-12-12 20:25
Sender: Francisco Figueiredo jr.


Ok, found what was the problem.

It is needed to have a transaction in order for the describe call be available for server processing. Anyway, I think that this is a problem of Postgresql server which shouldn't die. I will write to postgresql server people about that.

Anyway, I think we can make the describe messages more optimized as the parameter types won't change between calls, I think we could call the describe statement to get parameter return type when creating the NpgsqlBind message instead of when issuing the bind command. What do you think?
Date: 2005-12-12 19:48
Sender: Francisco Figueiredo jr.


Hi Hubert!

Thanks for your patch.

I could run tests here and they went well, but one:

if I try something like that:

NpgsqlCommand c = new NpgsqlCommand("a select command");
c.Prepare();
c.ExecuteReader();
c.ExecuteReader();

It crashes :(

What is really strange is that Postgresql server receives a signal 11 and closes all connections.

I'm using postgresql 8.0.3. Could you give it a try and let me know if you also get this error?

Thanks in advance.
Date: 2005-12-12 12:54
Sender: Hubert FONGARNAND

So this is my last version of the patch... With this patch, Npgsql ask the database for result's type... so now, only bytea value are transmitted in a binary format (when doing a select)...
Now it should work with all datatype
I've tested my patch and it seem's to work well!

Index: Npgsql/NpgsqlBind.cs
===================================================================
--- Npgsql/NpgsqlBind.cs (revision 54226)
+++ Npgsql/NpgsqlBind.cs (working copy)
@@ -73,6 +73,26 @@
return _portalName;
}
}
+
+ public String PreparedStatementName
+ {
+ get
+ {
+ return _preparedStatementName;
+ }
+ }
+
+ public Int16[] ResultFormatCodes
+ {
+ get
+ {
+ return _resultFormatCodes;
+ }
+ set
+ {
+ _resultFormatCodes=value;
+ }
+ }

public Int16[] ParameterFormatCodes
{
@@ -179,15 +199,11 @@
}
else
PGUtil.WriteInt16(outputStream, 0);
-
+
PGUtil.WriteInt16(outputStream, (Int16)_resultFormatCodes.Length);
for (i = 0; i < _resultFormatCodes.Length; i++)
- PGUtil.WriteInt16(outputStream, _resultFormatCodes[i]);
+ PGUtil.WriteInt16(outputStream, _resultFormatCodes[i]);

-
-
- }
-
}
}

Index: Npgsql/NpgsqlClosedState.cs
===================================================================
--- Npgsql/NpgsqlClosedState.cs (revision 54226)
+++ Npgsql/NpgsqlClosedState.cs (working copy)
@@ -110,7 +110,8 @@

}

- context.Stream = stream;
+ context.Stream = new BufferedStream(stream);
+

NpgsqlEventLog.LogMsg(resman, "Log_ConnectedTo", LogLevel.Normal, context.Host, context.Port);
ChangeState(context, NpgsqlConnectedState.Instance);
Index: Npgsql/NpgsqlCommand.cs
===================================================================
--- Npgsql/NpgsqlCommand.cs (revision 54226)
+++ Npgsql/NpgsqlCommand.cs (working copy)
@@ -598,14 +598,25 @@
if (parameters.Count != 0)
{
Object[] parameterValues = new Object[parameters.Count];
+ short[] formatCode=new short[parameters.Count];
+
for (Int32 i = 0; i < parameters.Count; i++)
{
// Do not quote strings, or escape existing quotes - this will be handled by the backend.
// DBNull or null values are returned as null.
// TODO: Would it be better to remove this null special handling out of ConvertToBackend??
- parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ if (parameters[i].TypeInfo.NpgsqlDbType!= NpgsqlDbType.Bytea)
+ {
+ parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ }
+ else
+ {
+ formatCode[i]=(Int16) FormatCode.Binary;
+ parameterValues[i]=(byte[])parameters[i].Value;
+ }
}
bind.ParameterValues = parameterValues;
+ bind.ParameterFormatCodes=formatCode;
}

Connector.Bind(bind);
@@ -701,6 +712,7 @@
connector.CheckErrorsAndNotifications();

bind = new NpgsqlBind("", planName, new Int16[] {0}, null, new Int16[] {0});
+ //bind = new NpgsqlBind(portalName, planName, new Int16[] {0}, null, new Int16[] {0});
}
}

Index: Npgsql/NpgsqlAsciiRow.cs
===================================================================
--- Npgsql/NpgsqlAsciiRow.cs (revision 54226)
+++ Npgsql/NpgsqlAsciiRow.cs (working copy)
@@ -130,15 +130,20 @@
data.Add(DBNull.Value);
continue;
}
+
+ NpgsqlRowDescriptionFieldData field_descr = row_desc[field_count];

- NpgsqlRowDescriptionFieldData field_descr = row_desc[field_count];
-
if (row_desc[field_count].format_code == FormatCode.Text)
{
string result = ReadStringFromStream(inputStream, field_value_size, decoder);
// Add them to the AsciiRow data.
data.Add(NpgsqlTypesHelper.ConvertBackendStringToSystemType(field_descr.type_info, result, field_descr.type_size, field_descr.type_modifier));
}
+ else if (row_desc[field_count].format_code == FormatCode.Binary)
+ {
+ Byte[] binary_data=ReadBytesFromStream(inputStream, field_value_size);
+ data.Add(NpgsqlTypesHelper.ConvertBackendBytesToSystemType(field_descr.type_info,binary_data,encoding,field_value_size, field_descr.type_modifier));
+ }
else
{
PGUtil.CheckedStreamRead(inputStream, _inputBuffer, 0, Math.Min(field_value_size, _inputBuffer.Length));
@@ -170,6 +175,29 @@
decoder.GetChars(_inputBuffer, 0, count, chars, 0);
return charCount;
}
+
+
+ private byte[] ReadBytesFromStream(Stream inputStream, int field_value_size)
+ {
+ byte[] binary_data=new byte[field_value_size];
+ int bytes_left = field_value_size;
+ if (field_value_size > _inputBuffer.Length)
+ {
+ int i=0;
+ while (bytes_left> READ_BUFFER_SIZE)
+ {
+ PGUtil.CheckedStreamRead(inputStream, _inputBuffer, 0, READ_BUFFER_SIZE);
+ _inputBuffer.CopyTo(binary_data,i*READ_BUFFER_SIZE);
+ i++;
+ bytes_left -= READ_BUFFER_SIZE;
+ }
+ }
+ PGUtil.CheckedStreamRead(inputStream,_inputBuffer, 0, bytes_left);
+ Int32 offset= field_value_size-bytes_left;
+ Array.Copy(_inputBuffer,0,binary_data,offset,bytes_left);
+ return binary_data;
+ }
+

private string ReadStringFromStream(Stream inputStream, int field_value_size, Decoder decoder)
{
Index: Npgsql/NpgsqlState.cs
===================================================================
--- Npgsql/NpgsqlState.cs (revision 54226)
+++ Npgsql/NpgsqlState.cs (working copy)
@@ -573,6 +573,16 @@

// Now wait for the AsciiRow messages.
break;
+
+ case NpgsqlMessageTypes_Ver_3.ParameterDescription:
+ // Do nothing,for instance, just read...
+ int lenght=PGUtil.ReadInt32(stream, inputBuffer);
+ int nb_param=PGUtil.ReadInt16(stream, inputBuffer);
+ for (int i=0;i<nb_param;i++)
+ {
+ int typeoid=PGUtil.ReadInt32(stream, inputBuffer);
+ }
+ break;

case NpgsqlMessageTypes_Ver_3.DataRow:
// This is the AsciiRow message.
@@ -703,6 +713,7 @@
}

break;
+
case NpgsqlMessageTypes_Ver_3.NoData :
// This nodata message may be generated by prepare commands issued with queries which doesn't return rows
// for example insert, update or delete.
Index: Npgsql/NpgsqlReadyState.cs
===================================================================
--- Npgsql/NpgsqlReadyState.cs (revision 54226)
+++ Npgsql/NpgsqlReadyState.cs (working copy)
@@ -106,7 +106,31 @@
public override void Bind(NpgsqlConnector context, NpgsqlBind bind)
{
NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Bind");
+ // Description...
+ NpgsqlDescribe describe= new NpgsqlDescribe('S', bind.PreparedStatementName);
BufferedStream stream = new BufferedStream(context.Stream);
+ describe.WriteToStream(stream, context.Encoding);
+ stream.Flush();
+ Sync(context);
+ Npgsql.NpgsqlRowDescription rowdesc=context.Mediator.LastRowDescription;
+ if (rowdesc!=null)
+ {
+ short[] resultformatcode=new short[rowdesc.NumFields];
+ for (int i=0;i<rowdesc.NumFields;i++)
+ {
+ Npgsql.NpgsqlRowDescriptionFieldData rowdescdata=rowdesc[i];
+ if (rowdescdata.type_info.NpgsqlDbType==NpgsqlTypes.NpgsqlDbType.Bytea)
+ {
+ // Binary format
+ resultformatcode[i]=1;
+ }
+ else
+ // Text Format
+ resultformatcode[i]=0;
+
+ }
+ bind.ResultFormatCodes=resultformatcode;
+ }
bind.WriteToStream(stream, context.Encoding);
stream.Flush();

Index: NpgsqlTypes/NpgsqlTypesHelper.cs
===================================================================
--- NpgsqlTypes/NpgsqlTypesHelper.cs (revision 54226)
+++ NpgsqlTypes/NpgsqlTypesHelper.cs (working copy)
@@ -97,7 +97,7 @@
{
NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "ConvertBackendBytesToStytemType");

- /*
+
// We are never guaranteed to know about every possible data type the server can send us.
// When we encounter an unknown type, we punt and return the data without modification.
if (TypeInfo == null)
@@ -105,26 +105,11 @@

switch (TypeInfo.NpgsqlDbType)
{
- case NpgsqlDbType.Binary:
+ case NpgsqlDbType.Bytea:
return data;
- case NpgsqlDbType.Boolean:
- return BitConverter.ToBoolean(data, 0);
- case NpgsqlDbType.DateTime:
- return DateTime.MinValue.AddTicks(IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0)));
-
- case NpgsqlDbType.Int16:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt16(data, 0));
- case NpgsqlDbType.Int32:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt32(data, 0));
- case NpgsqlDbType.Int64:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0));
- case NpgsqlDbType.String:
- case NpgsqlDbType.AnsiString:
- case NpgsqlDbType.StringFixedLength:
- return encoding.GetString(data, 0, fieldValueSize);
default:
throw new InvalidCastException("Type not supported in binary format");
- }*/
+ }

return null;
}
Date: 2005-12-12 09:52
Sender: Hubert FONGARNAND

Here's the new patch against the current version of npgsl...

Index: Npgsql/NpgsqlBind.cs
===================================================================
--- Npgsql/NpgsqlBind.cs (revision 54226)
+++ Npgsql/NpgsqlBind.cs (working copy)
@@ -182,7 +182,9 @@

PGUtil.WriteInt16(outputStream, (Int16)_resultFormatCodes.Length);
for (i = 0; i < _resultFormatCodes.Length; i++)
- PGUtil.WriteInt16(outputStream, _resultFormatCodes[i]);
+ //PGUtil.WriteInt16(outputStream, _resultFormatCodes[i]);
+ // Ask for binary result
+ PGUtil.WriteInt16(outputStream,1);



Index: Npgsql/NpgsqlClosedState.cs
===================================================================
--- Npgsql/NpgsqlClosedState.cs (revision 54226)
+++ Npgsql/NpgsqlClosedState.cs (working copy)
@@ -110,7 +110,8 @@

}

- context.Stream = stream;
+ context.Stream = new BufferedStream(stream);
+

NpgsqlEventLog.LogMsg(resman, "Log_ConnectedTo", LogLevel.Normal, context.Host, context.Port);
ChangeState(context, NpgsqlConnectedState.Instance);
Index: Npgsql/NpgsqlCommand.cs
===================================================================
--- Npgsql/NpgsqlCommand.cs (revision 54226)
+++ Npgsql/NpgsqlCommand.cs (working copy)
@@ -598,14 +598,25 @@
if (parameters.Count != 0)
{
Object[] parameterValues = new Object[parameters.Count];
+ short[] formatCode=new short[parameters.Count];
+
for (Int32 i = 0; i < parameters.Count; i++)
{
// Do not quote strings, or escape existing quotes - this will be handled by the backend.
// DBNull or null values are returned as null.
// TODO: Would it be better to remove this null special handling out of ConvertToBackend??
- parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ if (parameters[i].TypeInfo.NpgsqlDbType!= NpgsqlDbType.Bytea)
+ {
+ parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ }
+ else
+ {
+ formatCode[i]=(Int16) FormatCode.Binary;
+ parameterValues[i]=(byte[])parameters[i].Value;
+ }
}
bind.ParameterValues = parameterValues;
+ bind.ParameterFormatCodes=formatCode;
}

Connector.Bind(bind);
Index: Npgsql/NpgsqlAsciiRow.cs
===================================================================
--- Npgsql/NpgsqlAsciiRow.cs (revision 54226)
+++ Npgsql/NpgsqlAsciiRow.cs (working copy)
@@ -130,15 +130,20 @@
data.Add(DBNull.Value);
continue;
}
+
+ NpgsqlRowDescriptionFieldData field_descr = row_desc[field_count];

- NpgsqlRowDescriptionFieldData field_descr = row_desc[field_count];
-
if (row_desc[field_count].format_code == FormatCode.Text)
{
string result = ReadStringFromStream(inputStream, field_value_size, decoder);
// Add them to the AsciiRow data.
data.Add(NpgsqlTypesHelper.ConvertBackendStringToSystemType(field_descr.type_info, result, field_descr.type_size, field_descr.type_modifier));
}
+ else if (row_desc[field_count].format_code == FormatCode.Binary)
+ {
+ Byte[] binary_data=ReadBytesFromStream(inputStream, field_value_size);
+ data.Add(NpgsqlTypesHelper.ConvertBackendBytesToSystemType(field_descr.type_info,binary_data,encoding,field_value_size, field_descr.type_modifier));
+ }
else
{
PGUtil.CheckedStreamRead(inputStream, _inputBuffer, 0, Math.Min(field_value_size, _inputBuffer.Length));
@@ -170,6 +175,29 @@
decoder.GetChars(_inputBuffer, 0, count, chars, 0);
return charCount;
}
+
+
+ private byte[] ReadBytesFromStream(Stream inputStream, int field_value_size)
+ {
+ byte[] binary_data=new byte[field_value_size];
+ int bytes_left = field_value_size;
+ if (field_value_size > _inputBuffer.Length)
+ {
+ int i=0;
+ while (bytes_left> READ_BUFFER_SIZE)
+ {
+ PGUtil.CheckedStreamRead(inputStream, _inputBuffer, 0, READ_BUFFER_SIZE);
+ _inputBuffer.CopyTo(binary_data,i*READ_BUFFER_SIZE);
+ i++;
+ bytes_left -= READ_BUFFER_SIZE;
+ }
+ }
+ PGUtil.CheckedStreamRead(inputStream,_inputBuffer, 0, bytes_left);
+ Int32 offset= field_value_size-bytes_left;
+ Array.Copy(_inputBuffer,0,binary_data,offset,bytes_left);
+ return binary_data;
+ }
+

private string ReadStringFromStream(Stream inputStream, int field_value_size, Decoder decoder)
{
Index: NpgsqlTypes/NpgsqlTypesHelper.cs
===================================================================
--- NpgsqlTypes/NpgsqlTypesHelper.cs (revision 54226)
+++ NpgsqlTypes/NpgsqlTypesHelper.cs (working copy)
@@ -97,7 +97,7 @@
{
NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "ConvertBackendBytesToStytemType");

- /*
+
// We are never guaranteed to know about every possible data type the server can send us.
// When we encounter an unknown type, we punt and return the data without modification.
if (TypeInfo == null)
@@ -105,26 +105,22 @@

switch (TypeInfo.NpgsqlDbType)
{
- case NpgsqlDbType.Binary:
+ case NpgsqlDbType.Bytea:
return data;
case NpgsqlDbType.Boolean:
return BitConverter.ToBoolean(data, 0);
- case NpgsqlDbType.DateTime:
+ case NpgsqlDbType.Timestamp:
+ Console.WriteLine("data "+IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0)));
+
return DateTime.MinValue.AddTicks(IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0)));

- case NpgsqlDbType.Int16:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt16(data, 0));
- case NpgsqlDbType.Int32:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt32(data, 0));
- case NpgsqlDbType.Int64:
+ case NpgsqlDbType.Integer:
return IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0));
- case NpgsqlDbType.String:
- case NpgsqlDbType.AnsiString:
- case NpgsqlDbType.StringFixedLength:
+ case NpgsqlDbType.Varchar:
return encoding.GetString(data, 0, fieldValueSize);
default:
throw new InvalidCastException("Type not supported in binary format");
- }*/
+ }

return null;
}
Date: 2005-12-12 09:03
Sender: Hubert FONGARNAND

I've tested you'r improvment (with streaming). It reduced the memory use a little bit... But the memory load is always 5 time bigger than normally. And it doesn't reduce the network occupation (for a 5Mb file, there's 15Mb traffic (said ethereal)). I give you the new version of my patch (against 1.0beta as soon as can). I hope than it could be applied soon because we really need it, for our intranet
Thanks
Date: 2005-12-12 08:18
Sender: Hubert FONGARNAND

Just another question :
With other database connector (such as oracle, sql server...) you don't need to call Command.Prepare(), it's called automatically when you do a ExecuteReader (or fill) command. Sending parameters in the query is a very deprecated way... (and very insecure due to sql injection problems). Could npgsql call the prepare automatically too?
Date: 2005-12-12 05:28
Sender: Francisco Figueiredo jr.


Hi Hubert.

In order to do that, just pass the resultformatcodes array accordingly when creating the NpgsqlBind object.

I think NpgsqlBind could have an accessor for resultformatcode just like it has for parameterformatcode. So you could pass the information after creating NpgsqlBind.

I'm also working to add stream support for Npgsql. With your patch, you reduced in 5 times the memory used to send binary data. But if the binary data is still large, it will get a lot of memory. I'd like to add something like NpgsqlDbType.ByteStream or something like that. This way, parameters of this type would be Stream. This would allow Npgsql to read the stream in chuncks which would alliviate a lot the memory pressure even for large byte objects.

Anyway, thanks for your feedback and keep sending your patches so we can improve Npgsql!

Thanks Hubert!
Date: 2005-12-10 14:08
Sender: Hubert FONGARNAND

Hi Francisco,

With my patch, the client always ask the server to return binary data... It would be best to send ask binary data only for bytea data type (the corelab connector do this). But I don't know how to do this...
Date: 2005-12-09 13:28
Sender: Francisco Figueiredo jr.

Hi Hubert. I just updated Mono svn with latest Npgsql code. Revision 54161.

I'm now working on this issue. Thanks for your feedback.
Date: 2005-12-08 08:02
Sender: Hubert FONGARNAND

Yes i'm working with Npgsql from Mono... (because I can edit the file in MonoDevelop, since it support importing mono Makefile)

I'll upload an updated patch as soon as you update Npgsql in Mono!
Thanks
Date: 2005-12-07 14:05
Sender: Francisco Figueiredo jr.


Just another question. By the number of revision, I think you are using Npgsql from Mono svn. Is that right? I'm asking that because I'm going to update mono svn to latest 1.0beta1 version and I think there may be modifications to your patches.
Date: 2005-12-07 13:59
Sender: Francisco Figueiredo jr.


Hi Hubert!

Thank you for your patch! I'm looking at it. We also received another patch which also improves a lot the performance of npgsql :)

About your questions of returning binary data, I see that you already found it. :)

Date: 2005-12-07 11:19
Sender: Hubert FONGARNAND

With this patch, memory use when using bytea is 5 times reduced!!!!
Date: 2005-12-07 11:17
Sender: Hubert FONGARNAND

I've made a "first" patch in order to allow PostGreSQL to return binary value to Npgsql...

It works if you do a "prepare" before the query. For instance it's working with , varchar, int, and bytea (it doesn't work for timestamp)

Could you check if my patch is correct or help me to improve it
Thanks

Index: Npgsql/NpgsqlBind.cs
===================================================================
--- Npgsql/NpgsqlBind.cs (revision 54024)
+++ Npgsql/NpgsqlBind.cs (working copy)
@@ -182,7 +182,9 @@

PGUtil.WriteInt16(outputStream, (Int16)_resultFormatCodes.Length);
for (i = 0; i < _resultFormatCodes.Length; i++)
- PGUtil.WriteInt16(outputStream, _resultFormatCodes[i]);
+ //PGUtil.WriteInt16(outputStream, _resultFormatCodes[i]);
+ // Ask for binary result
+ PGUtil.WriteInt16(outputStream,1);



Index: Npgsql/NpgsqlCommand.cs
===================================================================
--- Npgsql/NpgsqlCommand.cs (revision 54024)
+++ Npgsql/NpgsqlCommand.cs (working copy)
@@ -598,14 +598,25 @@
if (parameters.Count != 0)
{
Object[] parameterValues = new Object[parameters.Count];
+ short[] formatCode=new short[parameters.Count];
+
for (Int32 i = 0; i < parameters.Count; i++)
{
// Do not quote strings, or escape existing quotes - this will be handled by the backend.
// DBNull or null values are returned as null.
// TODO: Would it be better to remove this null special handling out of ConvertToBackend??
- parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ if (parameters[i].TypeInfo.NpgsqlDbType!= NpgsqlDbType.Bytea)
+ {
+ parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ }
+ else
+ {
+ formatCode[i]=(Int16) FormatCode.Binary;
+ parameterValues[i]=(byte[])parameters[i].Value;
+ }
}
bind.ParameterValues = parameterValues;
+ bind.ParameterFormatCodes=formatCode;
}

Connector.Bind(bind);
Index: Npgsql/NpgsqlAsciiRow.cs
===================================================================
--- Npgsql/NpgsqlAsciiRow.cs (revision 54024)
+++ Npgsql/NpgsqlAsciiRow.cs (working copy)
@@ -130,6 +130,7 @@
charCount = decoder.GetCharCount(input_buffer, 0, bytes_left);
chars = new Char[charCount];
decoder.GetChars(input_buffer, 0, bytes_left, chars, 0);
+

result.Append(new String(chars));

@@ -155,7 +156,8 @@

for (Int16 field_count = 0; field_count < numCols; field_count++)
{
- Int32 field_value_size = PGUtil.ReadInt32(inputStream, input_buffer);
+ Int32 field_value_size = PGUtil.ReadInt32(inputStream, input_buffer);
+

// Check if this field is null
if (field_value_size == -1) // Null value
@@ -163,27 +165,35 @@
data.Add(DBNull.Value);
continue;
}
-
+ Byte[] binary_data=null;
+ if (row_desc[field_count].format_code == FormatCode.Binary)
+ binary_data=new Byte[field_value_size];
NpgsqlRowDescriptionFieldData field_descr = row_desc[field_count];
Int32 bytes_left = field_value_size;
- StringBuilder result = new StringBuilder();
+ StringBuilder result = new StringBuilder();
+ int i=0;

while (bytes_left > READ_BUFFER_SIZE)
{
-
// Now, read just the field value.
PGUtil.CheckedStreamRead(inputStream, input_buffer, 0, READ_BUFFER_SIZE);
+ if (row_desc[field_count].format_code == FormatCode.Text)
+ {

- // Read the bytes as string.
- //result.Append(new String(encoding.GetChars(input_buffer, 0, READ_BUFFER_SIZE)));
- charCount = decoder.GetCharCount(input_buffer, 0, READ_BUFFER_SIZE);
+ // Read the bytes as string.
+ //result.Append(new String(encoding.GetChars(input_buffer, 0, READ_BUFFER_SIZE)));
+ charCount = decoder.GetCharCount(input_buffer, 0, READ_BUFFER_SIZE);
+ chars = new Char[charCount];
+ decoder.GetChars(input_buffer, 0, READ_BUFFER_SIZE, chars, 0);
+ result.Append(new String(chars));
+ }
+ else
+ {
+ // Read the bytes as bytes...
+ input_buffer.CopyTo(binary_data,i*READ_BUFFER_SIZE);
+ i++;
+ }

- chars = new Char[charCount];
-
- decoder.GetChars(input_buffer, 0, READ_BUFFER_SIZE, chars, 0);
-
- result.Append(new String(chars));
-
bytes_left -= READ_BUFFER_SIZE;

// Now, read just the field value.
@@ -197,6 +207,7 @@

// Now, read just the field value.
PGUtil.CheckedStreamRead(inputStream, input_buffer, 0, bytes_left);
+ // Console.WriteLine("format_code :"+(FormatCode)row_desc[field_count].format_code);

if (row_desc[field_count].format_code == FormatCode.Text)
{
@@ -214,7 +225,12 @@
data.Add(NpgsqlTypesHelper.ConvertBackendStringToSystemType(field_descr.type_info, result.ToString(), field_descr.type_size, field_descr.type_modifier));

}
- else
+ else if (row_desc[field_count].format_code== FormatCode.Binary)
+ {
+ Int32 offset= field_value_size-bytes_left;
+ Array.Copy(input_buffer,0,binary_data,offset,bytes_left);
+ data.Add(NpgsqlTypesHelper.ConvertBackendBytesToSystemType(field_descr.type_info,binary_data,encoding,field_value_size, field_descr.type_modifier));
+ }else
// FIXME: input_buffer isn't holding all the field value. This code isn't handling binary data correctly.
data.Add(NpgsqlTypesHelper.ConvertBackendBytesToSystemType(field_descr.type_info, input_buffer, encoding, field_value_size, field_descr.type_modifier));
}
Index: NpgsqlTypes/NpgsqlTypesHelper.cs
===================================================================
--- NpgsqlTypes/NpgsqlTypesHelper.cs (revision 54024)
+++ NpgsqlTypes/NpgsqlTypesHelper.cs (working copy)
@@ -97,7 +97,7 @@
{
NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "ConvertBackendBytesToStytemType");

- /*
+
// We are never guaranteed to know about every possible data type the server can send us.
// When we encounter an unknown type, we punt and return the data without modification.
if (TypeInfo == null)
@@ -105,26 +105,22 @@

switch (TypeInfo.NpgsqlDbType)
{
- case NpgsqlDbType.Binary:
+ case NpgsqlDbType.Bytea:
return data;
case NpgsqlDbType.Boolean:
return BitConverter.ToBoolean(data, 0);
- case NpgsqlDbType.DateTime:
+ case NpgsqlDbType.Timestamp:
+ Console.WriteLine("data "+IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0)));
+
return DateTime.MinValue.AddTicks(IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0)));

- case NpgsqlDbType.Int16:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt16(data, 0));
- case NpgsqlDbType.Int32:
- return IPAddress.NetworkToHostOrder(BitConverter.ToInt32(data, 0));
- case NpgsqlDbType.Int64:
+ case NpgsqlDbType.Integer:
return IPAddress.NetworkToHostOrder(BitConverter.ToInt64(data, 0));
- case NpgsqlDbType.String:
- case NpgsqlDbType.AnsiString:
- case NpgsqlDbType.StringFixedLength:
+ case NpgsqlDbType.Varchar:
return encoding.GetString(data, 0, fieldValueSize);
default:
throw new InvalidCastException("Type not supported in binary format");
- }*/
+ }

return null;
}


Date: 2005-12-06 11:53
Sender: Hubert FONGARNAND

Now i've a memory problem when doing a SELECT with a large bytea...
It seems that postgresql return the bytea in a "quoted string" form to Npgsql... (5 time bigger on the network)
I've seen that with the corelab connector, the bytea is transmitted as a real binary!!!
Is there a way to have the same behaviour in Npgsql (when protocol V3 is specified)?
Date: 2005-12-06 11:49
Sender: Hubert FONGARNAND

Index: Npgsql/NpgsqlCommand.cs
===================================================================
--- Npgsql/NpgsqlCommand.cs (revision 53978)
+++ Npgsql/NpgsqlCommand.cs (working copy)
@@ -598,14 +598,25 @@
if (parameters.Count != 0)
{
Object[] parameterValues = new Object[parameters.Count];
+ short[] formatCode=new short[parameters.Count];
+
for (Int32 i = 0; i < parameters.Count; i++)
{
// Do not quote strings, or escape existing quotes - this will be handled by the backend.
// DBNull or null values are returned as null.
// TODO: Would it be better to remove this null special handling out of ConvertToBackend??
- parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ if (parameters[i].TypeInfo.NpgsqlDbType!= NpgsqlDbType.Bytea)
+ {
+ parameterValues[i] = parameters[i].TypeInfo.ConvertToBackend(parameters[i].Value, true);
+ }
+ else
+ {
+ formatCode[i]=(Int16) FormatCode.Binary;
+ parameterValues[i]=(byte[])parameters[i].Value;
+ }
}
bind.ParameterValues = parameterValues;
+ bind.ParameterFormatCodes=formatCode;
}

Connector.Bind(bind);
Date: 2005-12-06 11:49
Sender: Hubert FONGARNAND

I've made a little patch that allow Npgsql to send a bytea using real parameters (version3) (Command.prepare)


Date: 2005-12-06 10:53
Sender: Hubert FONGARNAND

Hi Francisco,

I've tried to use the Prepare method : It works, but with bytea parameters, the data must keep unescaped!!!
Npgsql send the bytea escaped even when using Prepare... I'm working on a patch to avoid it...
Date: 2005-12-05 19:55
Sender: Francisco Figueiredo jr.


Hi Hubert.

Real parameter support as you are saying is supported by calling the preapre method of Command. But it also needs some performance improvements.

It is exactly ToBinary() method which needs optimization. Npgsql needs to get all data to send the command to Postgresql. We need to change that to enable it to send data in blocks which would alliviate the stress of memory consumption.

We are working on it.

Thanks for your feedback.
Date: 2005-12-05 15:52
Sender: Hubert FONGARNAND

You're implementation of parameters (replacing parameters in the query) is always using strings instead of stringbuilder...
I'm looking replacing your strings by stringbuilders...
Date: 2005-12-05 11:03
Sender: Hubert FONGARNAND

We've seen that with the CoreLab connector, there's no problem with large bytea because it support real parameter (like in oracle)... So the binary value is not transmitted in the query in ASCII format? Is there a way to add support for real parameter in Npgsql
Date: 2005-12-05 10:46
Sender: Hubert FONGARNAND

After doing some testing, it seem's that this function is consuming much of memory...
in particular
res = new char[len * 5];
the memory needed for a bytea is 5 times the original object length!

internal abstract class BasicNativeToBackendTypeConverter
{
/// <summary>
/// Binary data.
/// </summary>
internal static String ToBinary(NpgsqlNativeTypeInfo TypeInfo, Object NativeData)
{
Byte[] byteArray = (Byte[])NativeData;
int len = byteArray.Length;
char[] res = new char[len * 5];

for (int i = 0, o = 0; i < len; ++i, o += 5)
{
byte item = byteArray[i];
res[o] = res[o + 1] = '\\';
res[o + 2] = (char)('0' + (7 & (item >> 6)));
res[o + 3] = (char)('0' + (7 & (item >> 3)));
res[o + 4] = (char)('0' + (7 & item));
}

return new String(res);
}

thanks
Date: 2005-12-05 10:31
Sender: Hubert FONGARNAND

I think this bug make Npgsql very unusable! Does someone know how to improve this behaviour (in the code)?
Or when do you think it will be fixed?
Thanks
Date: 2005-12-04 01:40
Sender: Francisco Figueiredo jr.


Hi.

This is a known issue with Npgsql. We have to optimize large bytea values.

Please, for while as an workaround, use the Large Object API. There is an example in the Npgsql User Manual in docs section.

Thanks for your interest in Npgsql and sorry for this problem.
Date: 2005-11-30 17:48
Sender: Arnaud Balandras

the attachment of the zipped file did not work.

Attached Files:

Changes:

Field Old Value Date By
status_idOpen2006-07-28 18:31fxjr
ResolutionNone2006-07-28 18:31fxjr
close_date2006-07-28 18:312006-07-28 18:31fxjr
Powered By FusionForge