View Issue Details

IDProjectCategoryView StatusLast Update
0017663FPCDatabasepublic2014-03-10 11:41
Reporteryang jixian Assigned ToMichael Van Canneyt  
PrioritynormalSeverityfeatureReproducibilityalways
Status resolvedResolutionno change required 
Summary0017663: TDBImage Bugs
Descriptionsigsegv while displaying image from sqlite database with tdbimage
TagsDatabase, dbimage, image, lazarus
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Relationships

related to 0013768 resolvedJesus Reyes Lazarus sigsegv while displaying image from sqlite database with tdbimage 
related to 0015858 closedVincent Snijders Lazarus TDBImage 
child of 0017748 resolvedMaxim Ganetsky Lazarus TDBImage Bugs: 

Activities

yang jixian

2010-10-19 04:22

reporter   ~0041909

Last edited: 2010-10-19 04:28

There is 8 bytes redundancy before bitmaps in biolife BLOB, dbdemos.7z fixed it.

Michael Van Canneyt

2010-10-19 16:51

administrator   ~0041915

Can you upload patches with regular zip please ? I don't have 7z

2010-10-20 09:23

 

dbctrls.patch (1,351 bytes)   
Index: dbctrls.pp
===================================================================
--- dbctrls.pp	(revision 27170)
+++ dbctrls.pp	(working copy)
@@ -997,6 +997,7 @@
     property DragCursor;
     property DragMode;
     property OnClick;
+    property OnDblClick;
     property OnDragDrop;
     property OnDragOver;
     property OnEndDrag;
Index: include/dbimage.inc
===================================================================
--- include/dbimage.inc	(revision 27170)
+++ include/dbimage.inc	(working copy)
@@ -133,12 +133,7 @@
 end;
 
 procedure TDBImage.LoadPicture;
-
 var s        : Tstream;
-    GraphExt : string;
-    gc       : TGraphicClass;
-    AGraphic : TGraphic;
-    
 begin
   if not FPictureLoaded then
     begin
@@ -160,22 +155,11 @@
           Picture.Clear;
           exit;
           end;
-        AGraphic := nil;
         try
-          GraphExt :=  s.ReadAnsiString;
-
-          gc := GetGraphicClassForFileExtension(GraphExt);
-          if assigned(gc) then
-            begin
-            AGraphic := gc.Create;
-            AGraphic.LoadFromStream(s);
-
-            Picture.Assign(AGraphic);
-            end;
+          Picture.LoadFromStream(S);
         finally
-          if assigned(AGraphic) then AGraphic.Free;
           s.Free;
-        end {try}
+        end
 
         end
       else
dbctrls.patch (1,351 bytes)   

yang jixian

2010-10-20 09:23

reporter   ~0041929

Last edited: 2010-10-20 09:24

Ok, the zip files come

2010-10-20 09:25

 

dbdemos.zip (825,124 bytes)

2010-10-20 09:26

 

dbpatchs.zip (10,782 bytes)

yang jixian

2010-10-20 09:29

reporter   ~0041930

dbctrls.patch fixed the following bug:
15858 The control doesn't have dblclick event.
0013768: sigsegv while displaying image from sqlite database with tdbimage

yang jixian

2010-11-03 15:00

reporter   ~0042720

memtable.pp is the memory table component.

Marco van de Voort

2011-06-13 16:23

manager   ~0049090

Last edited: 2011-06-13 16:52

STATUS:

!!!!Please don't ever run code beautifiers over sources before making changes. It makes figuring out changes very tedious. Specially since sqliteconn got quite a lot of patches over the last half year.!!!!

(1) The sqliteconn.patch seems to be already entirely committed. The only bit that is not committed (or has since been reverted?) is the entry of

 (n: 'GRAPHIC'; t: ftBlob),

to the fieldmap (and increase fieldmap's dimensions)

(2) dbctrls.patch the dblclick event has also been added to TDBImage.

(3) the dbimage.inc patch in dbctrls.patch has NOT yet been committed

(4) the memtable file has not been committed. TCustomBufDataset descendant which seems to have as main feat a kind of special copydataset functionality

Marco van de Voort

2011-06-13 16:40

manager   ~0049091

Last edited: 2011-06-13 17:31

The attached database really has a field of type "graphic" in table biolife, but also a "blob" with an image in it in the table animal. So the graphic blobtype might need the 8 byte skip and the blob one not?

According to sqlite type affinity rules, the field is a "numeric" field. (not matching any rules, and thus the default rule). ( !?!??!)

Marco van de Voort

2011-07-01 23:16

manager   ~0049564

Last edited: 2011-07-02 00:02

"Sqlite Expert" tool (personal edition available) seems to support "graphic" typed fields. It at least shows both kinds as images in this table.

Since such tools seem to do this in violation of the sqlite type affinity rules, it might be smartest to allow users to override sqlite ->sqldb fieldtype mappings

LacaK

2011-07-04 14:59

developer   ~0049650

I do not think, that we must support GRAPHIC as "special" datatype (and map GRAPHIC to ftBlob) . GRAPHIC is not sql standard nor is supported by other sql databases.
IMHO problem can be easy solved by declaring column "Graphic" in table "biolife" like BLOB (not GRAPHIC)

yang jixian

2011-07-06 05:18

reporter   ~0049709

I agree to LacaK2.

If there should be more information for graphic, there could be a new field which is supported by SQL standard to store them. Maybe we can detect whether there are 8 bytes or more before graphic data, but the efficience is decreased, especially when the component in the server side. And it is not the common use of SQL. Therefore it is not a good design for a database component.

LacaK

2011-07-06 07:20

developer   ~0049712

Or thinking more about it ... because fix (enhancement) is simple, we can for user convenience add:
 (n: 'GRAPHIC'; t: ftGraphic),
 (n: 'IMAGE'; t: ftGraphic), //as MS SQL Server

So map to ftGraphic not to ftBlob

(if you decide go this way, then you must also add ftGraphic in sqlite3conn.pp where is used ftBlob (in bindparams and loadfield methods))

Marco van de Voort

2011-07-06 14:59

manager   ~0049726

I'm hesitant to hardcode such mappings (which go against sqlite field affinity rules).

Rather I think a solution where a user can add such mappings is needed.

LacaK

2011-07-07 07:32

developer   ~0049740

OK then we can leave BLOB as is and let users to choose "BLOB" column datatype for their graphic, image data.
IMHO adding some event on TSQLite3Connection which will enable users to change default mapping is complication that is not worth it ;-)

yang jixian

2011-07-20 10:54

reporter   ~0050063

Turbo Delphi do not think that ftGraphic should be mapped to ftBlob.

The same ftGraphic in Turbo Delphi with eight bytes head can be read by TDBImage but without them the picture did not appear in TDBImage.

So if we want to be compatible with Delphi, there should be such eight bytes in the ftGraphic.

yang jixian

2011-07-20 11:06

reporter   ~0050064

By a ftgraphic type in Delphi, a picture type prefix is saved in front of the image stream.

yang jixian

2011-07-20 11:10

reporter   ~0050065

Exactly the first four bytes is the type prefix and the second four bytes is the image data length.

Marco van de Voort

2011-07-26 14:30

manager   ~0050177

The question is that sqlite does not provide a type that maps to ftgraphic AND specifies that all fieldtypes not in the SQLIte documentation should be considered numeric.

Thus a mapping from "GRAPHIC"(s) to ftgraphic breaks sqlite field rules.

This is why I think it is a good thing to only predefine the sqlite specified fieldtypes, and require custom mappings to be registered manually, so that any sqlite file that conforms to the sqlite's documentation's rules always loads.

Reinier Olislagers

2014-03-10 11:13

developer   ~0073585

- Agreed with 0049740;
- Since at least Laz 1.2, DBimage has supported Delphi compatible storage (actually arbitrary storage):
http://wiki.lazarus.freepascal.org/Lazarus_1.2.0_release_notes#TDBImage
so usrs can write their own handler for those graphic fields if needed.

suggest closing this issue, no change required.

Michael Van Canneyt

2014-03-10 11:41

administrator   ~0073591

As Reinier correctly remarks: the bug is solved with the introduction of the 2 events in TDBImage.

Issue History

Date Modified Username Field Change
2010-10-19 04:19 yang jixian New Issue
2010-10-19 04:19 yang jixian File Added: dbpatchs.7z
2010-10-19 04:22 yang jixian Note Added: 0041909
2010-10-19 04:26 yang jixian File Added: dbdemos.7z
2010-10-19 04:28 yang jixian Note Edited: 0041909
2010-10-19 16:51 Michael Van Canneyt Note Added: 0041915
2010-10-20 09:23 yang jixian File Added: dbctrls.patch
2010-10-20 09:23 yang jixian Note Added: 0041929
2010-10-20 09:24 yang jixian Note Edited: 0041929
2010-10-20 09:25 yang jixian File Added: dbdemos.zip
2010-10-20 09:26 yang jixian File Added: dbpatchs.zip
2010-10-20 09:29 yang jixian Note Added: 0041930
2010-10-29 10:08 Vincent Snijders LazTarget => 0.9.30
2010-10-29 10:08 Vincent Snijders Status new => acknowledged
2010-10-29 10:08 Vincent Snijders Target Version => 0.9.30
2010-10-29 10:11 Vincent Snijders Issue cloned: 0017748
2010-10-29 10:11 Vincent Snijders Relationship added related to 0017748
2010-10-29 10:12 Vincent Snijders Relationship replaced parent of 0017748
2010-10-29 10:12 Vincent Snijders Relationship replaced child of 0017748
2010-10-29 10:15 Vincent Snijders LazTarget 0.9.30 => -
2010-10-29 10:15 Vincent Snijders Target Version 0.9.30 =>
2010-10-29 10:15 Vincent Snijders Project Lazarus => FPC
2010-10-29 10:17 Vincent Snijders Relationship added related to 0013768
2010-10-29 22:02 Michael Van Canneyt Status acknowledged => assigned
2010-10-29 22:02 Michael Van Canneyt Assigned To => Michael Van Canneyt
2010-11-03 15:00 yang jixian Note Added: 0042720
2010-11-17 15:49 Felipe Monteiro de Carvalho Relationship added related to 0015858
2011-06-13 15:34 Marco van de Voort File Deleted: dbpatchs.7z
2011-06-13 15:34 Marco van de Voort File Deleted: dbdemos.7z
2011-06-13 16:23 Marco van de Voort Note Added: 0049090
2011-06-13 16:28 Marco van de Voort Note Edited: 0049090
2011-06-13 16:40 Marco van de Voort Note Added: 0049091
2011-06-13 16:42 Marco van de Voort Note Edited: 0049091
2011-06-13 16:48 Marco van de Voort Relationship added related to 0018779
2011-06-13 16:50 Marco van de Voort Relationship deleted related to 0018779
2011-06-13 16:52 Marco van de Voort Note Edited: 0049090
2011-06-13 17:31 Marco van de Voort Note Edited: 0049091
2011-07-01 23:16 Marco van de Voort Note Added: 0049564
2011-07-02 00:02 Marco van de Voort Note Edited: 0049564
2011-07-04 14:59 LacaK Note Added: 0049650
2011-07-06 05:18 yang jixian Note Added: 0049709
2011-07-06 07:20 LacaK Note Added: 0049712
2011-07-06 14:59 Marco van de Voort Note Added: 0049726
2011-07-07 07:32 LacaK Note Added: 0049740
2011-07-20 10:54 yang jixian Note Added: 0050063
2011-07-20 11:06 yang jixian Note Added: 0050064
2011-07-20 11:10 yang jixian Note Added: 0050065
2011-07-26 14:30 Marco van de Voort Note Added: 0050177
2014-03-10 11:13 Reinier Olislagers Note Added: 0073585
2014-03-10 11:15 Reinier Olislagers Tag Attached: Database
2014-03-10 11:15 Reinier Olislagers Tag Attached: dbimage
2014-03-10 11:15 Reinier Olislagers Tag Attached: image
2014-03-10 11:15 Reinier Olislagers Tag Attached: lazarus
2014-03-10 11:41 Michael Van Canneyt Note Added: 0073591
2014-03-10 11:41 Michael Van Canneyt Status assigned => resolved
2014-03-10 11:41 Michael Van Canneyt Resolution open => no change required